Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:
> > diff --git a/pkt-line.c b/pkt-line.c
> > index bd603f8..893dd3c 100644
> > --- a/pkt-line.c
> > +++ b/pkt-line.c
> > @@ -124,12 +124,14 @@ static int packet_length(const char *linelen)
> > int packet_read_line(int fd, char *buffer, unsigned size)
> > {
> > int len;
> > - char linelen[4];
> > + char linelen[5];
> >
> > safe_read(fd, linelen, 4);
> > len = packet_length(linelen);
> > - if (len < 0)
> > - die("protocol error: bad line length character");
> > + if (len < 0) {
> > + linelen[4] = '\0';
> > + die("protocol error: bad line length character: %s", linelen);
> > + }
>
> Since this is not called recursively, you can make linelen[] static
Sure. But it wasn't static before. It was stack allocated. Its a
5 byte stack allocation. Its not a lot of data to push onto the
stack, why does it suddenly matter that we make it static and charge
everyone for its memory instead of just those who really need it?
> and do
> without the NUL assignment; safe_read() won't read beyond 4 bytes anyway.
The NUL assignment isn't about safe_read(), its about making the
block of 4 bytes read a proper C-style string so we can safely pass
it down into the snprintf that is within die().
--
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html