On Mon, Feb 18, 2013 at 02:43:50AM -0800, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > The packet_read function reads from a descriptor.
> 
> Ah, so this introduces a new analagous helper that reads from
> a strbuf, to avoid the copy-from-async-procedure hack?

Not from a strbuf, but basically, yes.

> > +           ret = read_in_full(fd, dst, size);
> > +           if (ret < 0)
> > +                   die_errno("read error");
> 
> This is noisy about upstream pipe gone missing, which makes sense
> since this is transport-related.  Maybe that deserves a comment.

That is not new code; it is just reindented from the original safe_read.
But is it noisy about a missing pipe? We do not get EPIPE for reading.
We should just get a short read or EOF, both of which is handled later.

> > +           len = packet_read_from_buf(line, sizeof(line), &last->buf, 
> > &last->len);
> > +           if (len && line[len - 1] == '\n')
> > +                   len--;
> 
> Was anything guaranteeing that buffer.len < 1000 before this change?

No. That's discussed in point (3) of the "implications" in the commit
message.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to