On Wed, Apr 17, 2024 at 10:41:01AM +0200, Omar Polo wrote:
> > +const struct got_error *
> > +got_pkt_readlen(int *len, const char *str, int chattygot)
> > +{
> > [...]
> > +	errno = 0;
> > +	*len = strtol(str, &e, 16);
> > +	if (str[0] == '\0' || *e != '\0')
> > +		return got_error(GOT_ERR_BAD_PACKET);
> > +	if (errno == ERANGE || *len > INT_MAX || *len < INT_MIN)
> > +		return got_error_msg(GOT_ERR_BAD_PACKET, "bad pkt-line length");
> 
> I'm not sure this bit is correct.  You're storing a long inside a int,
> then check if it's bigger than INT_MAX or lesser than INT_MIN, which is
> impossible.

Right, I guess this broke during refactoring when I changed the type to int.
I wonder if we need to check anything at all since we know we parse at most
4 digits.

> 
> The original was
> 
> > -	errno = 0;
> > -	len = strtol(lenstr, &e, 16);
> > -	if (lenstr[0] == '\0' || *e != '\0')
> > -		return got_error(GOT_ERR_BAD_PACKET);
> > -	if (errno == ERANGE && (len == LONG_MAX || len == LONG_MIN))
> > -		return got_error_msg(GOT_ERR_BAD_PACKET, "bad packet length");
> > -	if (len > INT_MAX || len < INT_MIN)
> > -		return got_error_msg(GOT_ERR_BAD_PACKET, "bad packet length");
> 
> which is closer to the example code in strtol(3) (actually more readable
> than the EXAMPLE section.)