Page MenuHomeFreeBSD

Silence /usr/bin/top compiler warnings
ClosedPublic

Authored by rwestlun_gmail.com on May 19 2016, 8:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jul 14, 7:31 AM
Unknown Object (File)
Wed, Jul 10, 3:05 PM
Unknown Object (File)
Thu, Jul 4, 10:41 AM
Unknown Object (File)
Thu, Jul 4, 10:17 AM
Unknown Object (File)
Thu, Jun 27, 7:39 AM
Unknown Object (File)
Thu, Jun 27, 7:32 AM
Unknown Object (File)
Thu, Jun 27, 6:19 AM
Unknown Object (File)
Thu, Jun 27, 6:04 AM

Details

Summary

The /contrib/top code is no longer maintained upstream (last pulled 16 years ago). The
K&R-style followed by the code spews -Wimplicit-int and -Wreturn-type warnings,
among others. This diff silences 131 warnings with as little modification as possible.

The 5 warnings that remain are due to undeclared ncurses references. I didn't include
curses.h and term.h because there are several local functions and macros that conflict
with those declarations.

Test Plan

No functional change. Works for me.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rwestlun_gmail.com retitled this revision from to Silence /usr/bin/top compiler warnings.
rwestlun_gmail.com updated this object.
rwestlun_gmail.com edited the test plan for this revision. (Show Details)
rwestlun_gmail.com set the repository for this revision to rS FreeBSD src repository - subversion.

Please upload a diff with full context (generate the patch with -U999999).

Tangential, and please don't do it at the same time as these changes, but maybe top should be moved out of contrib/ if upstream is gone.

Regenerated with svn diff --diff-cmd=diff -x -U999999

cem added a reviewer: cem.

LGTM modulo comments.

This program could use ANSIfication of function definitions and a liberal dosing of style(9). That can wait for a later cleanup, though.

contrib/top/display.c
120 ↗(On Diff #16594)

spurious addition?

833–837 ↗(On Diff #16594)

__unused?

1012–1019 ↗(On Diff #16594)

__printflike(2, 3)? Would need a little reworking, though.

msgfmt should point to const.

a1/a2/a3 should be elided in favor of ... and stdarg.h. Then use vsnprintf instead of snprintf.

Lots of cleanup possible in this program.

contrib/top/display.h
14 ↗(On Diff #16594)

extra space between * and string

19–33 ↗(On Diff #16594)

Drop register in new code. It's not meaningful.

contrib/top/username.h
5–8 ↗(On Diff #16594)

Drop register in new code. It's not meaningful.

This revision is now accepted and ready to land.May 19 2016, 9:59 PM
In D6468#136976, @cem wrote:

This program could use ANSIfication of function definitions and a liberal dosing of style(9). That can wait for a later cleanup, though.

Agreed. I'm happy to do it, but I was wary of too many ANSI and style(9) changes without knowing whether they would be accepted. If someone commits this, I'll continue with the cleanup.

ngie added inline comments.
contrib/top/commands.c
31–36 ↗(On Diff #16594)

Header names are unsorted -- probably do this later.

47–48 ↗(On Diff #16594)

These could potentially just be labelled static in the definitions.

contrib/top/commands.h
1 ↗(On Diff #16594)

The copyright should be copied from contrib/top/commands.c ; just be sure to change the date and add your name ;).

contrib/top/machine.h
1 ↗(On Diff #16594)

Oy.. someone forgot to add the license when they added this file...

rwestlun_gmail.com edited edge metadata.

Updated to address comments:

  • Copy license to new header files.
  • Add __unused to a parameter.
  • Label two functions as static.
  • Remove extraneous space and newline.
  • Remove register from new code.

For the two header files that I created, I copied the copyright notices from the .c files. All the original headers (including machine.h) never had copyright notices in them in the upstream sources. I've left them as they are. If it's appropriate, I can add the copyright notices to them in later cleanup.

This revision now requires review to proceed.May 22 2016, 3:54 AM

There are a few more warnings that need to be cured:

/usr/src/svn/usr.bin/top/../../contrib/top/display.c:287:6: warning: implicit declaration of function 'tputs' is invalid in C99 [-Wimplicit-function-declaration]
            Move_to(x_lastpid, y_lastpid);
            ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.h:10:23: note: expanded from macro 'Move_to'
#define Move_to(x, y)   TCputs(tgoto(cursor_motion, x, y))
                        ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.h:8:21: note: expanded from macro 'TCputs'
#define TCputs(str)     tputs(str, 1, putstdout)
                        ^
1 warning generated.
cc -O2 -pipe -O2 -pipe -DHAVE_GETOPT -DHAVE_STRERROR -DORDER -I/usr/src/svn/usr.bin/top -I/usr/src/svn/usr.bin/top/../../contrib/top -I.   -g -MD  -MF.depend.machine.o -MTmachine.o -std=gnu99 -fstack-protector-strong -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion -Wno-unused-local-typedef -Wno-switch -Wno-switch-enum -Wno-knr-promoted-parameter -Wno-parentheses  -Qunused-arguments  -c /usr/src/svn/usr.bin/top/machine.c -o machine.o
cc -O2 -pipe -O2 -pipe -DHAVE_GETOPT -DHAVE_STRERROR -DORDER -I/usr/src/svn/usr.bin/top -I/usr/src/svn/usr.bin/top/../../contrib/top -I.   -g -MD  -MF.depend.screen.o -MTscreen.o -std=gnu99 -fstack-protector-strong -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion -Wno-unused-local-typedef -Wno-switch -Wno-switch-enum -Wno-knr-promoted-parameter -Wno-parentheses  -Qunused-arguments  -c /usr/src/svn/usr.bin/top/../../contrib/top/screen.c -o screen.o
/usr/src/svn/usr.bin/top/../../contrib/top/screen.c:136:19: warning: implicit declaration of function 'tgetent' is invalid in C99 [-Wimplicit-function-declaration]
    if ((status = tgetent(termcap_buf, term_name)) != 1)
                  ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.c:154:9: warning: implicit declaration of function 'tgetflag' is invalid in C99 [-Wimplicit-function-declaration]
    if (tgetflag("hc"))
        ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.c:161:26: warning: implicit declaration of function 'tgetnum' is invalid in C99 [-Wimplicit-function-declaration]
    if ((screen_length = tgetnum("li")) <= 0)
                         ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.c:318:2: warning: implicit declaration of function 'tputs' is invalid in C99 [-Wimplicit-function-declaration]
        putcap(terminal_init);
        ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.h:9:44: note: expanded from macro 'putcap'
#define putcap(str)     (void)((str) != NULL ? TCputs(str) : 0)
                                               ^
/usr/src/svn/usr.bin/top/../../contrib/top/screen.h:8:21: note: expanded from macro 'TCputs'
#define TCputs(str)     tputs(str, 1, putstdout)
                        ^
4 warnings generated.
ngie added a reviewer: ngie.
In D6468#137558, @ngie wrote:

There are a few more warnings that need to be cured:

Yeep. Fixing these is harder I noticed after adding curses.h and term.h. This is a good first step.. I'll commit the changes you provided.

This revision is now accepted and ready to land.May 22 2016, 4:07 AM
This revision was automatically updated to reflect the committed changes.