There are more than 5000 warnings issued during "make buildworld". Some of them are false positives, but some of them are sign of true errors. Nobody is upset by warnings due it's amount, so some errors remain uncorrected. I want to cleanup the code-base from warnings, so warnings will become "attention mark" again. usr.sbin/pppctl/pppctl.c:368: warning: 'fd' might be used uninitialized in this function 'fd' is initialized with no exception, so turn-off warning only How-To-Repeat: N/A
On 2004-09-12 04:38, Dan Lukes <dan@obluda.cz> wrote: > usr.sbin/pppctl/pppctl.c:368: warning: 'fd' might be used uninitialized in this function > 'fd' is initialized with no exception, so turn-off warning only It can't really. But this is not a good fix for the warning: > - int n, arg, fd, len, verbose, save_errno, hide1, hide1off, hide2; > + int n, arg, len, verbose, save_errno, hide1, hide1off, hide2; > + int fd = fd; /* init to avoid "might be used unitialized" warning 8/ fd = -1; would be a better initialization, since no valid descriptor can ever be negative and this will expose any bugs that using fd before a proper initialization can trigger. The following patch fixes a few other warnings too, and allows me to build pppctl with WARNS?=6 in today's 6.0-CURRENT: %%% Index: pppctl.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/pppctl/pppctl.c,v retrieving revision 1.32 diff -u -r1.32 pppctl.c --- pppctl.c 7 Dec 2003 08:39:29 -0000 1.32 +++ pppctl.c 12 Sep 2004 19:47:32 -0000 @@ -77,7 +77,7 @@ * How to use pppctl... */ static int -usage() +usage(void) { fprintf(stderr, "usage: pppctl [-v] [-t n] [-p passwd] " "Port|LocalSock [command[;command]...]\n"); @@ -93,7 +93,7 @@ * Handle the SIGALRM received due to a connect() timeout. */ static void -Timeout(int Sig) +Timeout(int Sig __unused) { TimedOut = 1; } @@ -103,10 +103,12 @@ * All the work is done in Receive() below. */ static char * -GetPrompt(EditLine *e) +GetPrompt(EditLine *e __unused) { + static char empty_prompt[] = ""; + if (prompt == NULL) - prompt = ""; + prompt = empty_prompt; return prompt; } @@ -119,6 +121,7 @@ Receive(int fd, int display) { static char Buffer[LINELEN]; + static char empty_prompt[] = ""; struct timeval t; int Result; char *last; @@ -173,7 +176,7 @@ Result = 0; break; } else - prompt = ""; + prompt = empty_prompt; if (len == sizeof Buffer - 1) { int flush; if ((last = strrchr(Buffer, '\n')) == NULL) @@ -202,7 +205,7 @@ * Note, this is a signal handler - be careful of what we do ! */ static void -InputHandler(int sig) +InputHandler(int sig __unused) { static char buf[LINELEN]; struct timeval t; @@ -365,9 +368,11 @@ main(int argc, char **argv) { struct sockaddr_un ifsun; - int n, arg, fd, len, verbose, save_errno, hide1, hide1off, hide2; + int fd = -1; + int n, arg, len, verbose, save_errno, hide1, hide1off, hide2; unsigned TimeoutVal; - char *DoneWord = "x", *next, *start; + char DoneWord[] = "x"; + char *next, *start; struct sigaction act, oact; void *thread_ret; pthread_t mon; @@ -431,7 +436,7 @@ hide1off, argv[harg]); else n = 0; - if (n < 0 || n >= sizeof title - pos) + if (n < 0 || (size_t)n >= sizeof title - pos) break; pos += n; } @@ -553,7 +558,7 @@ len = 0; Command[sizeof(Command)-1] = '\0'; for (arg++; arg < argc; arg++) { - if (len && len < sizeof(Command)-1) + if (len && (size_t) len < sizeof(Command)-1) strcpy(Command+len++, " "); strncpy(Command+len, argv[arg], sizeof(Command)-len-1); len += strlen(Command+len); %%%
On Sun, 12 Sep 2004, Giorgos Keramidas wrote: >> - int n, arg, fd, len, verbose, save_errno, hide1, hide1off, hide2; >> + int n, arg, len, verbose, save_errno, hide1, hide1off, hide2; >> + int fd = fd; /* init to avoid "might be used unitialized" warning 8/ > > fd = -1; would be a better initialization, since no valid descriptor can > ever be negative and this will expose any bugs that using fd before a > proper initialization can trigger. But unnecesarry over-initialisation is waste of resources. It's about decision ... I have no opinion which is better (generally).
On 2004-09-13 03:11, Dan Lukes <dan@obluda.cz> wrote: > On Sun, 12 Sep 2004, Giorgos Keramidas wrote: > >>- int n, arg, fd, len, verbose, save_errno, hide1, hide1off, hide2; > >>+ int n, arg, len, verbose, save_errno, hide1, hide1off, hide2; > >>+ int fd = fd; /* init to avoid "might be used unitialized" warning 8/ > > > >fd = -1; would be a better initialization, since no valid descriptor can > >ever be negative and this will expose any bugs that using fd before a > >proper initialization can trigger. > > But unnecesarry over-initialisation is waste of resources. > It's about decision ... Setting `int fd = fd' is also an initialiation that is a waste of resources. I see no reason why it's better than an initialization that also buys us some safety.
> Setting `int fd = fd' is also an initialiation that is a waste of > resources. Not true as it's optimized-out despite of O level. It's hack/information for compiler, not real assignment statement. > I see no reason why it's better than an initialization > that also buys us some safety. Agree. Lets initialise fd to -1 Dan
This patch isn't correct. I'll take this PR and fix the warnings.
I have no way to test this. Untaking.
Keyword: patch or patch-ready – in lieu of summary line prefix: [patch] * bulk change for the keyword * summary lines may be edited manually (not in bulk). Keyword descriptions and search interface: <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>