Follow-up Comment #3, bug #65452 (group groff):
Hi Branden,
[comment #2 comment #2:]
> commit d7b36a45fc3f49f7db82f5edd33c2a66696115e5 (HEAD -> master,
origin/master, origin/HEAD)
> Author: G. Branden Robinson <g.branden.robin...@gmail.com>
> Date: Wed Mar 13 14:50:42 2024 -0500
>
> [indxbib]: Mitigate Savannah #65452.
I see at least a bug being introduced in that commit. :)
@@ -343,16 +343,20 @@ 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);
+ if (ERANGE == errno)
+ fatal("argument to -%1 must be between %2 and %3", arg, min,
+ INT_MAX);
+ else if (n == 0 && ptr == arg)
+ fatal("argument to -%1 not an integer", opt);
-verbatim
You can't check errno after strtol(3) if you haven't cleared it before. You
should add
+verbatim+
errno = 0;
right before the strtol(3) call.
Here's The Right Way to call strtol(3):
<https://www.alejandro-colomar.es/src/alx/alx/lib/liba2i.git/tree/include/a2i/strtoi.h#n29>
----
BTW, where's the documentation for the xstrtol() you mentioned?
----
On another order of things, fatal() seems to be a '[[noreturn]]' thing, even
if it's not declared as such. 'else' is redundant, and you may prefer to omit
it. Among other things, it would have the benefit that this diff would have
been smaller (similar to the reasons why I defend semantic newlines).
----
And then there's some dead code in the commit:
+ if ((LONG_MAX > INT_MAX) && (n > INT_MAX))
+ fatal("argument to -%1 must be between %2 and %3", arg, min,
+ INT_MAX);
Since you already checked for ERANGE previously, there's no reason to check
for (LONG_MAX > INT_MAX). (n > INT_MAX) is already good enough, since in the
case long>int it will reject thing between INT_MAX and LONG_MAX, and in the
case they're equal, the test will always be false, but so will
LONG_MAX>INT_MAX. The real test that fixed that bug was the check for errno
(which is incorrect as I said earlier in this message, since you need to reset
errno before the call).
Cheers,
Alex
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?65452>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
