Page MenuHomeFreeBSD

top: fix warnings from clang/gcc
ClosedPublic

Authored by lidl on Apr 17 2018, 2:55 PM.

Details

Summary

Fix the remaining warnings emitted from clang/gcc for top.

Diff Detail

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

Event Timeline

lidl created this revision.Apr 17 2018, 2:55 PM
jhb accepted this revision.Apr 17 2018, 3:30 PM

Not sure if this will add conflicts for any future merges, but I don't think there's an upstream for our current top. (I think any upstream that might exist is probably quite a bit diverged already)

This revision is now accepted and ready to land.Apr 17 2018, 3:30 PM
cem added inline comments.Apr 17 2018, 5:48 PM
contrib/top/screen.h
31 ↗(On Diff #41573)

Why was either of these types changed? putchar() can only take character values and clearly the result is unused since the previous type was void.

lidl added inline comments.Apr 17 2018, 7:51 PM
contrib/top/screen.h
31 ↗(On Diff #41573)

Because putchar() is defined as taking an int, and returning an int. Since this function is being used as the function pointer in tputs(), it should return int, as that is what tputs() is expecting.

It was wrong before, when it took char, and had a void return type.

imp accepted this revision.Apr 17 2018, 8:00 PM

Seems sane. This code is quite old and long-of-tooth so it's a bit hard to read, so I didn't give it a super thorough going over...

contrib/top/screen.h
31 ↗(On Diff #41573)

Promotion rules allowed it to work (since the char was converted to an int before passing it in, and we weren't dealing with chars > 127), and a lack of checking the return code inside ncurses' tputs made whatever garbage would wind up there be ignored. So it was wrong, but that didn't matter: things still worked.

lidl added inline comments.Apr 17 2018, 8:05 PM
contrib/top/screen.h
31 ↗(On Diff #41573)

Yeah, I know about the promotion rules. Now it's correct, and it doesn't cause any compiler warnings.

(Why this rat-hole now? I noticed that there were warnings when looking at the latest ci.freebsd.org build for stable/11 on sparc64 and thought I would take a moment to fix them.)

What a tangled web we weave...

imp added inline comments.Apr 17 2018, 8:08 PM
contrib/top/screen.h
31 ↗(On Diff #41573)

The changes are good. Don't get me wrong. it was more to answer cem's HTF did this ever work comment...

This revision was automatically updated to reflect the committed changes.