Re: [PATCH 03/11] pkt-line: create gentle packet_read_line functions

[Kevin Wern <kevin.m.wern@gmail.com>]

On Fri, Sep 16, 2016 at 03:17:28PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.wern@gmail.com> writes:
> 
> >  	/* And complain if we didn't get enough bytes to satisfy the read. */
> >  	if (ret < size) {
> > -		if (options & PACKET_READ_GENTLE_ON_EOF)
> > +		if (options & (PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ALL))
> >  			return -1;
> 
> The name _ALL suggested to me that there may be multiple "under this
> condition, be gentle", "under that condition, be gentle", and _ALL
> is used as a catch-all "under any condition, be gentle".  If you
> defined _ALL symbol to have all GENTLE bits on, this line could have
> become
> 
> 	if (options & PACKET_READ_GENTLE_ALL)
> 
> > @@ -205,15 +209,23 @@ int packet_read(int fd, char **src_buf, size_t *src_len,
> >  	if (ret < 0)
> >  		return ret;
> >  	len = packet_length(linelen);
> > -	if (len < 0)
> > +	if (len < 0) {
> > +		if (options & PACKET_READ_GENTLE_ALL)
> > +			return -1;
> 
> On the other hand, however, you do want to die here when only
> GENTLE_ON_EOF is set.
> 
> Taking the above two observations together, I'd have to say that
> _ALL is probably a misnomer.  I agree with a need for a flag with
> the behaviour you defined in this patch, though.

OK, my thought is either:
	- Come up with a name for a flag, or flags, for the other cases (to
	  check in the function, i.e. PACKET_READ_GENTLE_INVALID), and still
	  pass in PACKET_READ_GENTLE_ALL, which is all those bits on plus
	  *_GENTLE_ON_EOF.
	- Come up with a better name for this single flag, like
	  PACKET_READ_DONT_DIE ... only better.

What would you suggest here?