Re: [PATCH 07/26] connect: convert get_remote_heads to use struct packet_reader

From: Brandon Williams <bmwill@google.com>
Date: Jan 9, 2018, 19:09 UTC
Message-ID: <20180109190931.GA151395@google.com>
In-Reply-To: <20180109102735.a6d3330325aef428fde87700@google.com>

On 01/09, Jonathan Tan wrote:

Show 13 quoted lines

> On Tue,  2 Jan 2018 16:18:09 -0800
> Brandon Williams <bmwill@google.com> wrote:
> 
> > -	while ((len = read_remote_ref(in, &src_buf, &src_len, &responded))) {
> > +	while (state != EXPECTING_DONE) {
> > +		switch (packet_reader_read(&reader)) {
> > +		case PACKET_READ_EOF:
> > +			die_initial_contact(1);
> > +		case PACKET_READ_NORMAL:
> > +			len = reader.pktlen;
> > +			if (len > 4 && skip_prefix(packet_buffer, "ERR ", &arg))
> 
> This should be a field in reader, not the global packet_buffer, I think.

Thanks for catching that.

Show 5 quoted lines

> 
> Also, I did a search of usages of packet_buffer, and there are just a
> few of them - it might be worthwhile to eliminate it, and have each
> component using it allocate its own buffer. But this can be done in a
> separate patch set.

I'll go through and eliminate the references to packet_buffer by passing in the buffer explicitly.

Show 7 quoted lines

> 
> > @@ -269,6 +284,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
> >  			if (process_shallow(len, shallow_points))
> >  				break;
> >  			die("protocol error: unexpected '%s'", packet_buffer);
> 
> Here too.

-- 
Brandon Williams