From: Tobias Heider Subject: Re: move got_pkt_readlen() to separate function To: Omar Polo Cc: gameoftrees@openbsd.org Date: Thu, 18 Apr 2024 16:51:37 +0200 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.)