From: Jeff King <peff@peff.net>
Date: Fri, 15 May 2020 20:56:32 GMT
Subject: Re: [PATCH 4/6] pkt-line: extern packet_length()
Message-ID: <20200515205632.GB115445@coredump.intra.peff.net>
In-Reply-To: <891a39c853ce3669b6167dc9ad8a2328e4321a9e.1589393036.git.liu.denton@gmail.com>

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.

-Peff