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

Conversation

Projects
None yet
4 participants
@morrone
Copy link
Contributor

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

This comment has been minimized.

Copy link

coveralls commented Jan 6, 2017

Coverage Status

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

@codecov-io

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

morrone commented Jan 6, 2017

OK, I'll fix that up.

Convert to inttypes.h conversion specifiers
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

This comment has been minimized.

Copy link

coveralls commented Jan 6, 2017

Coverage Status

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

@garlick

This comment has been minimized.

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

4 checks passed

codecov/patch 88.46% of diff hit (target 76.09%)
Details
codecov/project Absolute coverage decreased by -0.03% but relative coverage increased by +12.36% compared to 7788f20
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 76.343%
Details

@garlick garlick removed the review label Jan 6, 2017

@morrone morrone deleted the morrone:inttypes_cleanup branch Feb 25, 2017

@grondo grondo referenced this pull request Mar 28, 2017

Closed

0.7.0 Release Notes #1019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.