On Tue, Mar 12, 2024 at 11:22:32PM +0100, Alejandro Colomar wrote:
> On Tue, Mar 12, 2024 at 05:05:31PM -0500, G. Branden Robinson wrote:
> > Hi Alex,
> >
> > I forgot about this; fortunately, Dave reminded me.
>
> Hi Branden! (Hi Dave!)
>
> > At 2024-01-14T20:19:18+0100, Alejandro Colomar wrote:
> > > I see some code calling strtol(3) that I suspect won't behave well in
> > > some systems:
> > >
> > > $ grepc -tfd check_integer_arg .
> > > ./src/utils/indxbib/indxbib.cpp:static void check_integer_arg(char opt,
> > > const char *arg, int min, int *res)
> > > {
> > > char *ptr;
> > > long n = strtol(arg, &ptr, 10);
> > > if (n == 0 && ptr == arg)
> > > error("argument to -%1 not an integer", opt);
> > > else if (n < min)
> > > error("argument to -%1 must not be less than %2", opt, min);
> > > else {
> > > if (n > INT_MAX)
> > > error("argument to -%1 greater than maximum integer", opt);
> > > else if (*ptr != '\0')
> > > error("junk after integer argument to -%1", opt);
> > > *res = int(n);
> > > }
> > > }
> > >
> > >
> > > I think these tests miss some corner cases:
> > >
> > > - If INT_MAX==LONG_MAX, then n>INT_MAX is impossible, but strtol(3)
> > > will return LONG_MAX and errno ERANGE for values greater than that.
> > > groff is silently accepting input >LONG_MAX in those systems, and
> > > silently saturating it to LONG_MAX (INT_MAX).
> >
> > Yes--I forgot about systems where sizeof (int) == sizeof (long).
> >
> > So I reckon I'll throw the `long long` type and `strtoll()` at it. We
> > claim to require a C99 compiler already.
>
> That's still a problem on ILP64, ain't it? :)
> Not that I like such systems, but Paul Eggert reminded me of their
> existence when I suggested a similar fix for a similar problem some time
> ago.
>
> You'll need to just use a better API. strtoi(3), provided by the BSDs,
> and by libbsd on non-BSD systems, is a better one. It had a bug until
> earlier this year, when I fixed it, so you may want to avoid it. Then
> you may become the first user of liba2i[1], or roll your own wrapper
> (hopefully compatible with liba2i).
>
> [1] <https://git.kernel.org/pub/scm/libs/liba2i/liba2i.git/>
>
> If you want to use it, please let me know; I'm still working on
> the build system. The source code, however, is tested, and I'd
> say it's good.
>
> > The call sites (and some context) are as follows.
> >
> > 79 int hash_table_size = DEFAULT_HASH_TABLE_SIZE;
> >
> > 147 case 'h':
> > 148 {
> > 149 int requested_hash_table_size;
> > 150 check_integer_arg('h', optarg, 1, &requested_hash_table_size);
> > 151 hash_table_size = requested_hash_table_size;
> > 152 if ((hash_table_size > 2) && (hash_table_size % 2) == 0)
> > 153 hash_table_size++;
> > 154 while (!is_prime(hash_table_size))
> > 155 hash_table_size += 2;
> > 156 if (hash_table_size != requested_hash_table_size)
> > 157 warning("requested hash table size %1 is not prime: using %2"
> > 158 " instead", optarg, hash_table_size);
> > 159 }
> > 160 break;
> >
> > You may see another problem here. We accept '1' as an argument, but
> > then pass it to a function called `is_prime()`...which fails an
> > assertion on that input. Whoops.
>
> Hmmm, yeah. So you could raise it to 3, and then also drop the >2 test.Whoops, no. The liwer bound is 2, and you still need the >2 test. -- <https://www.alejandro-colomar.es/> Looking for a remote C programming job at the moment.
signature.asc
Description: PGP signature
