Fix style nits in strings
ClosedPublic
Actions

Authored by oshogbo on Nov 18 2018, 3:07 PM.

Details

Reviewers

allanjude
cem
markj
emaste
jkoshy_users.sourceforge.net

Commits

rS367466: Sync up elftoolchain with head, except for the capsicum-related commits,
rS340746: strings: fix style nits

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

oshogbo created this revision.Nov 18 2018, 3:07 PM

cem accepted this revision.Nov 18 2018, 11:41 PM

cem added inline comments.

contrib/elftoolchain/strings/strings.c
386–395	I'd drop `(void)` from `printf()`. Checking return value of printf is extremely uncommon.
401	Maybe s/1/true/ too
447	same (drop void)

This revision is now accepted and ready to land.Nov 18 2018, 11:41 PM

Changes look reasonable to me, but we will want to make sure changes go upstream too so adding @kaiw and @jkoshy_users.sourceforge.net for a heads-up. (I can commit the style changes upstream.)

Thanks for these cleanups.

contrib/elftoolchain/strings/strings.c
386–395	It may be easier for code maintainers if we treat `printf()` the same as any other function whose return value is being ignored.

@oshogbo if you commit this to FreeBSD I'll grab the diff and commit to ELF Tool Chain.

cem added inline comments.Nov 21 2018, 4:37 PM

contrib/elftoolchain/strings/strings.c
386–395	I don't think a consistency argument makes sense when in this same function you have 3 unchecked `putchar()` missing `(void)` and 1 other unchecked `printf()` missing `(void)`.

Closed by commit rS340746: strings: fix style nits (authored by oshogbo). · Explain WhyNov 21 2018, 9:48 PM

This revision was automatically updated to reflect the committed changes.

oshogbo marked an inline comment as done.

Herald added a subscriber: imp. · View Herald TranscriptNov 21 2018, 9:48 PM

yuripv added a subscriber: yuripv.Nov 21 2018, 10:05 PM

yuripv added inline comments.

head/contrib/elftoolchain/strings/strings.c
353 ↗	(On Diff #50906)	A bit of post-commit noise; style(9): "Forever loops are done with for's, not while's."
398 ↗	(On Diff #50906)	Same use-for-not-while nit.

jkoshy_users.sourceforge.net added inline comments.Nov 21 2018, 11:08 PM

contrib/elftoolchain/strings/strings.c
386–395	Those could be changed if we are aiming for FreeBSD style(9) compliance. We didn't have a defined style when the project started. What would be nice would be a style(9)-aware linter for code -- is there such a thing?