On Fri, Sep 16, 2016 at 03:17:28PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.wern@xxxxxxxxx> 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?