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

Convert to inttypes.h conversion specifiers #940

Merged
merged 1 commit into from
Jan 6, 2017
Merged

Convert to inttypes.h conversion specifiers #940

merged 1 commit into from
Jan 6, 2017

Conversation

morrone
Copy link
Contributor

@morrone morrone commented Jan 6, 2017

Change the printf conversion specifiers to use the portable
macros from inittypes.h. For instance, instead of guessing %lu for
a uint32_t, use the PRIu32 macro.

Also change the length modifier for size_t types to "z", which is
an explicit printf length modifier for size_t and ssize_t arguments.

Finally, any %m conversion specifiers are replaced with explicit
%s and strerror(errno). The %m is convenient, but it is explicitly
listed as a Glibc extension and therefore less portable.

Closes #394

@garlick garlick added the review label Jan 6, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0009%) to 76.359% when pulling 9dc4acb on morrone:inttypes_cleanup into 3cf4c55 on flux-framework:master.

@codecov-io
Copy link

codecov-io commented Jan 6, 2017

Current coverage is 76.05% (diff: 88.46%)

Merging #940 into master will decrease coverage by 0.03%

@@             master       #940   diff @@
==========================================
  Files           149        149          
  Lines         25951      25951          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          19747      19738     -9   
- Misses         6204       6213     +9   
  Partials          0          0          
Diff Coverage File Path
0% src/common/libutil/popen2.c
0% src/common/libsubprocess/subprocess.c
••••• 50% src/common/libcompat/reactor.c
•••••••••• 100% src/modules/kvs/libkvs.c
•••••••••• 100% src/cmd/flux-ping.c
•••••••••• 100% src/broker/module.c
•••••••••• 100% src/common/libutil/nodeset.c
•••••••••• 100% src/modules/libjsc/jstatctl.c
•••••••••• 100% src/modules/connector-local/local.c
•••••••••• 100% src/modules/wreck/wrexecd.c

Review all 20 files changed

Powered by Codecov. Last update 7788f20...3a7c99b

@garlick
Copy link
Member

garlick commented Jan 6, 2017

Cool! I didn't know about %z.

One nit, the log_err() calls in subprocess.c should drop the %m/strerror bit since log_err() already appends a colon and the strerror.

@morrone
Copy link
Contributor Author

morrone commented Jan 6, 2017

OK, I'll fix that up.

Change the printf conversion specifiers to use the portable
macros from inittypes.h.  For instance, instead of guessing %lu for
a uint32_t, use the PRIu32 macro.

Also change the length modifier for size_t types to "z", which is
an explicit printf length modifier for size_t and ssize_t arguments.

Remove uses of The %m conversion specifier.  While it is convenient,
it is explicitly listed as a Glibc extension and therefore less
portable.  In subprocess.c, the %m was already redundant with the
error string appened by log_err(), so they were simply removed.  The
one final instance of %m in popen2.c was converted to an explicit
%s and strerr(errno).

Closes #394
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 76.343% when pulling 3a7c99b on morrone:inttypes_cleanup into 7788f20 on flux-framework:master.

@garlick
Copy link
Member

garlick commented Jan 6, 2017

That's got it, thank you.

@garlick garlick merged commit f172cc3 into flux-framework:master Jan 6, 2017
@garlick garlick removed the review label Jan 6, 2017
@morrone morrone deleted the inttypes_cleanup branch February 25, 2017 01:34
@grondo grondo mentioned this pull request Mar 28, 2017
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