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

build: fix newer compiler warnings about enum passed to va_start #1184

Merged
merged 3 commits into from
Sep 8, 2017

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Sep 8, 2017

This PR fixes newer compiler warnings that complain about passing default type promoted objects as an argument to va_start. In both places this is done in the current code, an enum is passed as the last argument before the ellipsis in a variadic function, and I guess this could cause undefined behavior depending on how the enum is promoted.

The fix for now is to simply change the argument to an integer.

This should fix current failures in travis, since the warning causes a fatal error with -Werror

Change prototype of subprocess_manager_set() to avoid using an enum
as the argument passed to va_start(), as this causes warnings on newer
compilers which complain about

      passing an object that undergoes default argument promotion
      to 'va_start' has undefined behavior [-Werror,-Wvarargs]

The enum undergoes default promotion, and thus it may cause undefined
behavior. Therefore, change the enum argument to an integer to avoid
this warning.
Change prototype of optparse_get() and optparse_set() to avoid
passing an enum as the argument to va_start() in these variadic
functions. Use of enum causes newer compiler to issue warnings of
the form:

 passing an object that undergoes default argument promotion
 to 'va_start' has undefined behavior [-Werror,-Wvarargs]

Change the enum argument in these functions to an integer to suppress
the warning and avoid potential undefined behavior.
@grondo
Copy link
Contributor Author

grondo commented Sep 8, 2017

Looks like this PR might turn into "fix travis-ci on latest trusty image" -- the libfaketime compile is also failing with the latest compilers so I'll have to fix that one here as well.

Update to latest upstream version of libfaketime. Also, libfaketime
doesn't appear to work with recent versions of clang. Force use
of GCC.
@grondo grondo force-pushed the va_start-avoid-enum branch from aa28d71 to d206431 Compare September 8, 2017 18:12
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 78.226% when pulling d206431 on grondo:va_start-avoid-enum into 9c860d0 on flux-framework:master.

@codecov-io
Copy link

codecov-io commented Sep 8, 2017

Codecov Report

Merging #1184 into master will increase coverage by 0.05%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1184      +/-   ##
==========================================
+ Coverage   77.82%   77.87%   +0.05%     
==========================================
  Files         158      158              
  Lines       29255    29254       -1     
==========================================
+ Hits        22767    22783      +16     
+ Misses       6488     6471      -17
Impacted Files Coverage Δ
src/common/libsubprocess/subprocess.c 81.68% <100%> (ø) ⬆️
src/common/liboptparse/optparse.c 88.44% <50%> (ø) ⬆️
src/common/libflux/request.c 88.46% <0%> (-1.29%) ⬇️
src/common/libflux/handle.c 83.66% <0%> (-0.5%) ⬇️
src/common/libcompat/handle.c 84.94% <0%> (-0.28%) ⬇️
src/common/libflux/message.c 81.05% <0%> (-0.24%) ⬇️
src/common/libflux/response.c 84.55% <0%> (+0.81%) ⬆️
src/common/libflux/future.c 89.6% <0%> (+0.99%) ⬆️
src/broker/content-cache.c 74.56% <0%> (+1.29%) ⬆️
src/broker/module.c 84.67% <0%> (+1.39%) ⬆️
... and 2 more

@garlick
Copy link
Member

garlick commented Sep 8, 2017

Thanks for running this down.

@garlick garlick merged commit ea1f316 into flux-framework:master Sep 8, 2017
@grondo grondo deleted the va_start-avoid-enum branch September 8, 2017 21:10
@grondo grondo mentioned this pull request May 10, 2018
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.

4 participants