On Mon, Aug 1, 2022 at 6:18 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Matheus Tavares <matheus.bernardino@xxxxxx> writes:
>
> > + die("expected key '%s', got '%.*s'",
> > + key, orig_size, orig_buf);
> > +
> > + buf[size] = '\0';
>
> I find this assignment somewhat strange, but primarily because it
> uses the updated buf[size] that ought to be pointing at the same
> byte as the original buf[size]. Is this necessary because buf[size]
> upon the entry to this function does not necessarily have NUL there?
>
> Reading ahead,
>
> * packet_key_val_read() feeds the buffer taken from
> packet_read_line_gently(), so buf[size] should be NUL terminated
> already.
>
> * read_capabilities() feeds the buffer taken from
> packet_read_line(), so buf[size] should be NUL terminated
> already.
>
> > + return buf;
> > +}
>
> And the caller gets the byte position that begins the <value> part.
Good point. I'll remove the buf[size] = '\0' assignment.
> > + if (entry && !entry->requested) {
> > + entry->requested = 1;
> > + } else if (!entry && always_delay) {
> > + add_delay_entry(pathname, 1, 1);
> > + }
>
> These are unnecessary {} around single statement blocks, but let's
> let it pass in a test helper.
> > [...]
> > + die("Unknown message '%s'", buf);
> > + }
> > + }
> > +
> > +
> > + read_packetized_to_strbuf(0, &input, 0);
>
> I do not see a need for double blank lines above.
Oops, I will fix these too. Thanks.
