* getopt.c warnings patch
@ 2020-05-27 22:09 Paul J. Lucas
2020-05-27 23:59 ` Bruno Haible
0 siblings, 1 reply; 2+ messages in thread
From: Paul J. Lucas @ 2020-05-27 22:09 UTC (permalink / raw)
To: bug-gnulib
The getopt.c file generates the following warnings from Apple’s gcc (Apple clang version 11.0.3 (clang-1103.0.32.62)):
--------------------------------------------------------------------------------
getopt.c:208:21: warning: implicit conversion changes signedness: 'long' to
'size_t' (aka 'unsigned long') [-Wsign-conversion]
namelen = nameend - d->__nextchar;
~ ~~~~~~~~^~~~~~~~~~~~~~~
getopt.c:255:34: warning: implicit conversion changes signedness: 'int' to
'unsigned long' [-Wsign-conversion]
else if ((ambig_set = malloc (n_options)) == NULL)
~~~~~~ ^~~~~~~~~
getopt.c:369:16: warning: variable 'option_index' may be uninitialized when used
here [-Wconditional-uninitialized]
*longind = option_index;
^~~~~~~~~~~~
getopt.c:204:19: note: initialize the variable 'option_index' to silence this
warning
int option_index;
^
= 0
3 warnings generated.
--------------------------------------------------------------------------------
when compiled with these warnings enabled:
-Wall -Wcast-align -Wcomma -Wconditional-type-mismatch -Wconditional-uninitialized -Wconversion -Wextra -Wfloat-equal -Wfor-loop-analysis -Widiomatic-parentheses -Wimplicit-fallthrough -Wlogical-op-parentheses -Wnewline-eof -Wno-unknown-warning-option -Wredundant-decls -Wshadow -Wshift-sign-overflow -Wsign-compare -Wsign-conversion -Wsometimes-uninitialized -Wstring-conversion -Wuninitialized -Wunreachable-code-break -Wunreachable-code -Wunused -Wwrite-strings
Below is a patch that fixes all these warnings.
- Paul
--- lib/getopt.c.ORIG 2020-05-27 14:45:22.000000000 -0700
+++ lib/getopt.c 2020-05-27 14:57:42.000000000 -0700
@@ -201,11 +201,11 @@
const struct option *p;
const struct option *pfound = NULL;
int n_options;
- int option_index;
+ int option_index = 0;
for (nameend = d->__nextchar; *nameend && *nameend != '='; nameend++)
/* Do nothing. */ ;
- namelen = nameend - d->__nextchar;
+ namelen = (size_t)(nameend - d->__nextchar);
/* First look for an exact match, counting the options as a side
effect. */
@@ -252,7 +252,7 @@
{
if (__libc_use_alloca (n_options))
ambig_set = alloca (n_options);
- else if ((ambig_set = malloc (n_options)) == NULL)
+ else if ((ambig_set = malloc ((size_t)n_options)) == NULL)
/* Fall back to simpler error message. */
ambig_fallback = 1;
else
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: getopt.c warnings patch
2020-05-27 22:09 getopt.c warnings patch Paul J. Lucas
@ 2020-05-27 23:59 ` Bruno Haible
0 siblings, 0 replies; 2+ messages in thread
From: Bruno Haible @ 2020-05-27 23:59 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul J. Lucas
Hi,
Paul J. Lucas wrote:
> The getopt.c file generates the following warnings from Apple’s gcc (Apple clang version 11.0.3 (clang-1103.0.32.62)):
>
> --------------------------------------------------------------------------------
> getopt.c:208:21: warning: implicit conversion changes signedness: 'long' to
> 'size_t' (aka 'unsigned long') [-Wsign-conversion]
> namelen = nameend - d->__nextchar;
> ~ ~~~~~~~~^~~~~~~~~~~~~~~
> getopt.c:255:34: warning: implicit conversion changes signedness: 'int' to
> 'unsigned long' [-Wsign-conversion]
> else if ((ambig_set = malloc (n_options)) == NULL)
> ~~~~~~ ^~~~~~~~~
The obvious "fix" for these warnings is to introduce a cast. But such casts
would decrease the robustness of the code. As I wrote in [1], such explicit
casts introduce bugs when the standards change or some platform is not 100%
standards compliant.
Therefore it is best to ignore warnings of this type. That's what gnulib does,
through the file build-aux/gcc-warning.spec, when you use the
gl_MANYWARN_ALL_GCC macro.
> getopt.c:369:16: warning: variable 'option_index' may be uninitialized when used
> here [-Wconditional-uninitialized]
> *longind = option_index;
> ^~~~~~~~~~~~
> getopt.c:204:19: note: initialize the variable 'option_index' to silence this
> warning
> int option_index;
> ^
> = 0
Here the code is copying an uninitialized value, if pfound == NULL. But this is
harmless, because
1) The documentation of _getopt_internal_r says that
"LONGIND returns the index in LONGOPT of the long-named option found.
It is only valid when a long-named option has been found by the most
recent call."
2) valgrind does not complain about copying an uninitialized value, if it ends
up being unused.
Bruno
[1] https://bugs.llvm.org/show_bug.cgi?id=46025
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-27 23:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 22:09 getopt.c warnings patch Paul J. Lucas
2020-05-27 23:59 ` Bruno Haible
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).