Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require warning-free compilation #61

Merged
merged 10 commits into from
Feb 27, 2022
Merged

Conversation

garlick
Copy link
Member

@garlick garlick commented Feb 26, 2022

This PR fixes all known compiler warnings and re-enables -Wall -Werror so we catch future ones in CI.

It also enables automake "maintainer mode", because that's handy.

Problem: makefiles are not rebuild when autotools source
files are modified.

Add the [enable] argument to the AM_MAINTAINER_MODE macro call.

Add AC_CONFIG_MACRO_DIR([config]) to avoid errors when config-missing
is automatically run.
Problem: gcc10 emits "warning: suggest parentheses around assignment
used as truth value" when building liblsd.

In file included from hostlist.c:43:
hostlist.c: In function ‘hostlist_new’:
hostlist.c:1066:12: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
 1066 |     assert(new->magic = HOSTLIST_MAGIC);
      |            ^~~

Relax warning cflags for liblsd directory.
Problem: gcc10 emits "warning: comparison between pointer and zero
character constant" in hostlist.c.

hostlist.c: In function ‘_next_tok’:
hostlist.c:372:48: warning: comparison between pointer and zero character constant [-Wpointer-compare]
  372 |     while (**str != '\0' && strchr(sep, **str) != '\0')
      |                                                ^~

Compare strchr reuslt with NULL instead of '\0'.
Problem: gcc10 complains of stringop-truncation.

hostlist.c: In function ‘_hostlist_create_bracketed.constprop’:
hostlist.c:1515:9: warning: ‘strncpy’ specified bound 1024 equals destination size [-Wstringop-truncation]
 1515 |         strncpy(cur_tok, tok, 1024);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Use sizeof - 1 as copy size.
Problem: compilation of flex-generated C code compilains of unused
function.

parse_lex.c:1766:16: warning: ‘input’ defined but not used [-Wunused-function]
 1766 |     static int input  (void)
      |                ^~~~~

Define YY_NO_INPUT in lex source to suppress function definition.
Problem: gcc10 complains of stringop-truncation error when compliling
generated lexer.

parse_lex.c: In function ‘yylex’:
parse_lex.l:105:9: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
  105 |         yylval[len] = '\0';
      |         ^~~~~~~~~~~~~~~~~~~
parse_lex.l:102:15: note: length computed here
  102 |         yylval = xmalloc(len + 1);
      |               ^~~~~~~~~~~~~~~~~~

This code looks like it just wants to do a xstrdup(), so do that instead.
Problem: gcc10 complains of snprintf format-truncation when
compiling a test program.

vpcd.c:279:51: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  279 |         snprintf(prompt, sizeof(prompt), "%d vpc> ", seq);
      |                                                   ^
vpcd.c:279:9: note: ‘snprintf’ output between 8 and 17 bytes into a destination of size 16
  279 |         snprintf(prompt, sizeof(prompt), "%d vpc> ", seq);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Increase buffer size.
Problem: gcc9 complains of maybe-unitialized warning when
compiling test programs.

ilom.c: In function ‘main’:
ilom.c:192:9: error: ‘authenticated’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  192 |         switch (authenticated) {
      |         ^~~~~~
ilom.c:175:9: note: ‘authenticated’ was declared here
  175 |     int authenticated;
      |         ^~~~~~~~~~~~~

lom.c: In function ‘main’:
lom.c:135:9: error: ‘authenticated’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
         switch (authenticated) {
         ^~~~~~
lom.c:118:9: note: ‘authenticated’ was declared here
     int authenticated;
         ^~~~~~~~~~~~~

Initialize variable to zero.
Problem: compiler warnings are not detected during CI builds.

Re-enable -Wall -Werror so that warnings are treated as errors.
Problem: @GCCWARN@ is substituted even when the compiler is clang.

Import the AX_COMPILER_VENDOR macro and use it to test whether the
compiler vendor is clang or gnu.

Change the warning flags macro name to WARNING_CFLAGS, then conditionally
set it to different values for clang vs gnu.  Tell clang not to complain
about unknown-warning-option errors.  Although this is not currently an
issue in powerman, it is in other projects, so might as well.
Copy link
Member

@grondo grondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@garlick
Copy link
Member Author

garlick commented Feb 27, 2022

Thanks for the review on this one too. I'll go ahead and merge.

@garlick garlick merged commit e869ec2 into chaos:master Feb 27, 2022
@garlick garlick deleted the fix_warnings branch February 27, 2022 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants