Page MenuHomeFreeBSD

Fix style nits in strings
ClosedPublic

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

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem added inline comments.
contrib/elftoolchain/strings/strings.c
386–395 ↗(On Diff #50560)

I'd drop (void) from printf(). Checking return value of printf is extremely uncommon.

401 ↗(On Diff #50560)

Maybe s/1/true/ too

447 ↗(On Diff #50560)

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 ↗(On Diff #50560)

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.

contrib/elftoolchain/strings/strings.c
386–395 ↗(On Diff #50560)

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).

This revision was automatically updated to reflect the committed changes.
oshogbo marked an inline comment as done.
yuripv added inline comments.
head/contrib/elftoolchain/strings/strings.c
353

A bit of post-commit noise; style(9): "Forever loops are done with for's, not while's."

398

Same use-for-not-while nit.

contrib/elftoolchain/strings/strings.c
386–395 ↗(On Diff #50560)

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?

contrib/elftoolchain/strings/strings.c
386–395 ↗(On Diff #50560)

No freebsd style(9) aware linters that I know of, sorry.

contrib/elftoolchain/strings/strings.c
386–395 ↗(On Diff #50560)

None exist. And generally do what's best for the authors, especially for the authors of external software like elftoolchain...