Fix -Wmaybe-uninitialized warning in xdr.c

Message ID 20140708175919.GL609@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar July 8, 2014, 5:59 p.m. UTC
  Hi,

While we're at fixing build warnings, here's one unnecessary warning
that can be fixed fairly easily.  The SIZE variable is never actually
use uninitialized, but the compiler cannot make that out and thinks
(correctly) that there is a potential for accessing SIZE without
initializing it.  Make this safe by initializing SIZE to 0.

Tested on x86_64.

Siddhesh

	* sunrpc/xdr.c (xdr_string): Initialize SIZE to 0.
  

Comments

Adhemerval Zanella Netto July 8, 2014, 6:01 p.m. UTC | #1
On 08-07-2014 14:59, Siddhesh Poyarekar wrote:
> Hi,
>
> While we're at fixing build warnings, here's one unnecessary warning
> that can be fixed fairly easily.  The SIZE variable is never actually
> use uninitialized, but the compiler cannot make that out and thinks
> (correctly) that there is a potential for accessing SIZE without
> initializing it.  Make this safe by initializing SIZE to 0.
>
> Tested on x86_64.
>
> Siddhesh
>
> 	* sunrpc/xdr.c (xdr_string): Initialize SIZE to 0.
>
> diff --git a/sunrpc/xdr.c b/sunrpc/xdr.c
> index b3f96ca..129abd8 100644
> --- a/sunrpc/xdr.c
> +++ b/sunrpc/xdr.c
> @@ -739,7 +739,7 @@ xdr_string (xdrs, cpp, maxsize)
>       u_int maxsize;
>  {
>    char *sp = *cpp;	/* sp is the actual string pointer */
> -  u_int size;
> +  u_int size = 0;
>    u_int nodesize;
>  
>    /*
Seems trivial enough.
  
Roland McGrath July 9, 2014, 7:39 p.m. UTC | #2
This is fine in this case (and I noticed you already committed it).  But
I'll point out that in the past the policy was explicitly not to make this
sort of change.  That is, when a change is necessary to suppress some
compiler warning, but the change results in worse code (here, a dead
store), then we favor optimal code over warning-free code.  Sometimes the
source code can be rejiggered such that the generated code is the same (or
better) but the compiler groks it better and doesn't generate the bogus
warning.  Sometimes that's impossible and we just have to accept that we
are smarter than the compiler is.

Nowadays, we probably want to use a balancing test of the performance vs
the ease-of-maintenance improvement of eliminating warnings.  It seems
almost certain that we don't care about the cost of the dead store in
xdr_string.  But IMHO it should always be an explicit decision to trade off
after careful consideration of the particular case.  It should never be a
default-OK to pessimize the code to work around the compiler's lack of
intelligence.  Certainly any change made for that reason should have
comments saying that the initialization is not actually desirable but was
added to silence a compiler warning.


Thanks,
Roland
  

Patch

diff --git a/sunrpc/xdr.c b/sunrpc/xdr.c
index b3f96ca..129abd8 100644
--- a/sunrpc/xdr.c
+++ b/sunrpc/xdr.c
@@ -739,7 +739,7 @@  xdr_string (xdrs, cpp, maxsize)
      u_int maxsize;
 {
   char *sp = *cpp;	/* sp is the actual string pointer */
-  u_int size;
+  u_int size = 0;
   u_int nodesize;
 
   /*