On Fri, May 15, 2020 at 04:56:32PM -0400, Jeff King wrote:
> On Wed, May 13, 2020 at 02:04:56PM -0400, Denton Liu wrote:
>
> > +/*
> > + * Reads a packetized line and returns the length header of the packet.
> > + */
> > +int packet_length(const char *linelen);
>
> If this is becoming public, I think we need to talk a bit more about:
>
> - what is linelen; it's not a NUL-terminated string, which I would
> have expected from the prototype. It must be at least 4 chars, and
> doesn't need terminated.
>
> - what happens on error; it looks like we return -1
>
> I think the prototype would be more clear to me as:
>
> int packet_length(const char *line, size_t len)
> {
> if (len < 4)
> return -1;
> }
>
> which makes it clear that this is a sized buffer. But since nobody
> should ever be passing anything except "4", it may be overkill. I'd be
> happy enough with a comment.
Oh, and obviously:
int packet_length(const char linelen[4]);
would be descriptive, but I think falls afoul of C pointer/array
weirdness.
-Peff
