[Bug 6716] Found code mistakes using cppcheck

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=6716

rickards <rickardpajobb at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |

--- Comment #6 from rickards <rickardpajobb at gmail.com> 2013-12-02 12:46:47 UTC ---
Hi

I want to begin by thanking you for busybox , and it's fun to be able to
contribute to such a great project :)

But some commentators , I feel is in place..


(In reply to comment #3)
> You did find some bugs, and I'm applying some parts of the patch.
> 
> The parts I don't apply are:
> 
> --- a/applets/applet_tables.c
> +++ b/applets/applet_tables.c
> @@ -150,6 +150,7 @@ int main(int argc, char **argv)
>                         if (!fp)
>                                 return 1;
>                         fputs(line_new, fp);
> +                       fclose(fp);
>                 }
>         }
> 
> Yes, we intentionally don't close the file. Exiting will do it for us.

Although the program is closed and we do not "have to" close the file pointer,
we should surely do so when we found the "error"...  I think.

> --- a/networking/udhcp/d6_dhcpc.c
> +++ b/networking/udhcp/d6_dhcpc.c
> @@ -749,7 +749,8 @@ print_inetname(const struct sockaddr *from)
>                         n = xmalloc_sockaddr2host_noport((struct
> sockaddr*)from);
>                 }
>                 printf("  %s (%s)", (n ? n : ina), ina);
> -               free(n);
> +               if(n)
> +                       free(n);
>         }
>         free(ina);
>  }
> 
> free(NULL) is valid.

I know you can make free ( NULL) now, but actually thought that this could be a
problem on older systems , so this was an attempt on my part to be a little
backwards compatible ..
But after a bit of googling I have not found anything to suggest that this ever
been a problem. Nice, then I learned something :)


> -    VALUE *l = l; /* silence gcc */
> -    VALUE *v = v; /* silence gcc */
> +    VALUE *l = 0;
> +    VALUE *v = 0;
> 
> ...and tons of similar "fixes". Please see the comments on the lines you are
> changing.
> WE DO THAT ON PURPOSE: we know that variables are uninitialized. We know it's
> ok in these cases.
> "int i = i;" trick is used to make compiler stop complaining.


I know char * X = X; allows variable is uninitialized while eliminating
warnings from gcc.
The question is whether this is worth doing so, in pure assembler code it saves
one command ..!
Is that the optimization degree busybox tend?
There is for example, MISRA C standard where one should always put all the
variables to an initial value because this is one of the major sources of error
in C.
Not least when one gradually make even more changes to existing code.
Or another way to look at this. If there be only one of these values, which
were the slightest chance of being used uninitialized it would not be worth
this minimal optimization ...?
I've found 20, this was why I start to fix all of them in the first place.

coreutils/expr.c : 326] :  (error) Uninitialized variable :  l
coreutils/expr.c : 327] :  (error) Uninitialized variable :  v
coreutils/test.c : 633] :  (error) Uninitialized variable :  i
miscutils/chrt.c : 65] :  (error) Uninitialized variable :  priority
miscutils/taskset.c : 89] :  (error) Uninitialized variable :  aff
miscutils/ubi_tools.c : 109] :  (error) Uninitialized variable :  size_bytes
networking/httpd.c : 2469] :  (error) Uninitialized variable :  server_socket
networking/nc_bloaty.c : 590] :  (error) Uninitialized variable :  zp
networking/nc_bloaty.c : 591] :  (error) Uninitialized variable :  np
networking/tcpudp.c : 210] :  (error) Uninitialized variable :  len_per_host
networking/tcpudp.c : 217] :  (error) Uninitialized variable :  remote_hostname
networking/tcpudp.c : 218] :  (error) Uninitialized variable :  remote_addr
networking/udhcp/d6_dhcpc.c : 1059] :  (error) Uninitialized variable : 
timestamp_before_wait
networking/udhcp/dhcpc.c : 1399] :  (error) Uninitialized variable : 
timestamp_before_wait
networking/udhcp/domain_codec.c : 33] :  (error) Uninitialized variable :  ret
networking/zcip.c : 160] :  (error) Uninitialized variable :  addr
shell/hush.c : 4954] :  (error) Uninitialized variable :  exp_save
shell/hush.c : 4956] :  (error) Uninitialized variable :  exp_word
util-linux/fbset.c : 421] :  (error) Uninitialized variable :  mode
util-linux/mdev.c : 624] :  (error) Uninitialized variable :  aliaslink

-- 
Configure bugmail: https://bugs.busybox.net/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.