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

job-manager: fix uninitialized variable warning in jobtap.c #3481

Merged
merged 1 commit into from Jan 25, 2021

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Jan 25, 2021

Some compilers complain about va_list ap being used uninitialized in jobtap_get_priority().

Split the plugin args creation function into two parts, one that take a va_list and one that doesn't so that an uninitialized va_list doesn't need to be used when the calling function doesn't take variable arguments.

Problem: In job-manager/jobtap.c:jobtap_get_priority() a NULL va_list
and fmt is passed to the function jobtap_args_vcreate().  Though the
va_list argument is only used if fmt != NULL, some compilers complain
that `va_list ap` may be used uninitialized in this function.

Instead of being lazy, split the function that creates the plugin
arguments into two -- one that takes va_list argument and one that
does not. This is cleaner code anyway.
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

LGTM

Edit: looks like @garlick beat me by 22 seconds :P

@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #3481 (dd18b76) into master (03dc09c) will decrease coverage by 0.00%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master    #3481      +/-   ##
==========================================
- Coverage   82.43%   82.43%   -0.01%     
==========================================
  Files         319      319              
  Lines       48369    48374       +5     
==========================================
+ Hits        39874    39875       +1     
- Misses       8495     8499       +4     
Impacted Files Coverage Δ
src/modules/job-manager/jobtap.c 82.64% <77.77%> (-0.53%) ⬇️
src/modules/job-archive/job-archive.c 59.03% <0.00%> (-0.81%) ⬇️
src/common/libflux/message.c 83.68% <0.00%> (-0.13%) ⬇️
src/broker/modservice.c 72.18% <0.00%> (+0.75%) ⬆️

@grondo
Copy link
Contributor Author

grondo commented Jan 25, 2021

Thanks! We'll be carrying this one as a patch in the v0.23 RPMS 🤦

@mergify mergify bot merged commit 9689726 into flux-framework:master Jan 25, 2021
@grondo grondo deleted the jobtap-uninit-error branch January 25, 2021 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants