Summary: | [patch] cleanup of the usr.sbin/pppctl code | ||||||
---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Dan Lukes <dan> | ||||
Component: | bin | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
Status: | Open --- | ||||||
Severity: | Affects Some People | CC: | ngie | ||||
Priority: | Normal | Keywords: | patch | ||||
Version: | 5.3-BETA3 | ||||||
Hardware: | Any | ||||||
OS: | Any | ||||||
Attachments: |
|
Description
Dan Lukes
2004-09-12 03:40:30 UTC
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> |