Re: [PATCH v2] Refactor recv_sideband()

[Jeff King <peff@peff.net>]

On Tue, Jun 14, 2016 at 02:25:42PM -0700, Junio C Hamano wrote:

> > Also, reorganize the overall control flow, remove some superfluous
> > variables and replace a custom implementation of strpbrk() with a call
> > to the standard C library function.
> 
> I find that calling the loop "a custom implementation" is a bit
> unfair.  The original tried to avoid looking beyond "len", but in
> the updated code because you have buf[len] = '\0' to terminate the
> line, and because you pass LARGE_PACKET_MAX to packet_read() while
> your buf[] allocates one more byte, you can use strpbrk() here
> safely. Which would mean "a custom implementation" was done for a
> reason.

Knowing we have a NUL at the end makes strpbrk() safe to use (as opposed
to walking off the end of the buffer). But what about the opposite case,
when there are embedded NULs in the data?

I think this case is already fairly broken by the use of "%s" specifiers
later in the function, and I doubt it is all that useful. So I am OK if
the answer is "we don't care, and do not even consider it a bug that
should be fixed".

-Peff