Hi Alex, I forgot about this; fortunately, Dave reminded me.
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.
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.
164 case 'k':
165 check_integer_arg('k', optarg, 1, &max_keys_per_item);
166 break;
167 case 'l':
168 check_integer_arg('l', optarg, 0, &shortest_len);
169 break;
170 case 'n':
171 check_integer_arg('n', optarg, 0, &n_ignore_words);
172 break;
176 case 't':
177 check_integer_arg('t', optarg, 1, &truncate_len);
178 break;
> - If min==INT_MIN==LONG_MIN, then a similar thing happens for
> underflow.
On the other hand, this will never happen. All of the call sites use a
static literal minimum value, and that is always 0 or 1...or, soon, 2.
Let me know if I overlooked something (again).
Regards,
Branden
signature.asc
Description: PGP signature
