Re: [PATCH 4/6] pkt-line: extern packet_length()
- From
Jeff King <peff@peff.net>
- Date
- May 15, 2020, 20:57 UTC
- Message-ID
- <20200515205717.GC115445@coredump.intra.peff.net>
- In-Reply-To
- <20200515205632.GB115445@coredump.intra.peff.net>
On Fri, May 15, 2020 at 04:56:32PM -0400, Jeff King wrote:
Show 26 quoted lines
> 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