Re: [PATCH] Rename read_pipe() with read_fd() and make its buffer nul-terminated.
- From
- Carlos Rica <jasampler@gmail.com>
- Date
- Jul 18, 2007, 15:40 UTC
- Message-ID
- <1b46aba20707180840l6c12fecfy4c610fe3a1a36c5e@mail.gmail.com>
- In-Reply-To
- <7vps2q6tjf.fsf@assigned-by-dhcp.cox.net>
2007/7/18, Junio C Hamano <gitster@pobox.com>:
Show 16 quoted lines
>
> > unsigned long size = *return_size;
> > ssize_t iret;
> > unsigned long off = 0;
> >
> > + if (!buf || size <= 1) {
> > + size = alloc_nr(size);
> > + buf = xrealloc(buf, size);
> > + }
> > +
>
> Hmph. The reason this is not "!size" is because you would want
> more than one. As your plan is to use this mostly for the -F
> option of "tag/commit", I suspect using a bit larger default,
> such as 80 (just a line), or probably 1k (most log messages
> would fit in such a buffer), would be more practical.The true reason is because I before tried using (size-1) instead of (size) in xread(..., size - off), and then I forgot to remove that condition. Sorry.
Show 21 quoted lines
>
> > do {
> > iret = xread(fd, buf + off, size - off);
> > if (iret > 0) {
> > off += iret;
> > if (off == size) {
> > - size *= 2;
> > + size = alloc_nr(size);
> > buf = xrealloc(buf, size);
> > }
> > }
> > } while (iret > 0);
> >
> > + if (off == size)
> > + buf = xrealloc(buf, size + 1);
> > + buf[off] = '\0';
> > +
>
> I wonder if doing xread(... (size-1) - off) in the loop would
> ensure (off <= size-1) here. You also would need to update the
> realloc condition inside loop if you do so.I have chosen that solution to leave the code easy to read, but the (size-1) version will avoid that additional (but non-frequent) realloc.
I will resend the fixed patch along with the builtin-tag.c one.