top: fix warnings from clang/gcc
ClosedPublic
Actions

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

Details

Reviewers

eadler
emaste
jhb
ngie
cem
imp

Commits

rS332671: top: fix warnings from clang/gcc

Summary

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

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• lidl created this revision.Apr 17 2018, 2:55 PM

Herald added a subscriber: imp. · View Herald TranscriptApr 17 2018, 2:55 PM

Harbormaster completed remote builds in B16187: Diff 41573.Apr 17 2018, 2:55 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

go for it

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.

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