Message ID | 20140708175919.GL609@spoyarek.pnq.redhat.com |
---|---|
State | Committed |
Headers |
Received: (qmail 18792 invoked by alias); 8 Jul 2014 17:59:31 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 18652 invoked by uid 89); 8 Jul 2014 17:59:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Date: Tue, 8 Jul 2014 23:29:19 +0530 From: Siddhesh Poyarekar <siddhesh@redhat.com> To: libc-alpha@sourceware.org Subject: Fix -Wmaybe-uninitialized warning in xdr.c Message-ID: <20140708175919.GL609@spoyarek.pnq.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="37cJpJlYZwAfNbm5" Content-Disposition: inline User-Agent: Mutt/1.5.22.1-rc1 (2013-10-16) |
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
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.
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
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; /*