From patchwork Thu Nov 13 21:41:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joseph Myers X-Patchwork-Id: 3719 Received: (qmail 21100 invoked by alias); 13 Nov 2014 21:41:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 21087 invoked by uid 89); 13 Nov 2014 21:41:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Date: Thu, 13 Nov 2014 21:41:48 +0000 From: Joseph Myers To: Roland McGrath CC: "GNU C. Library" Subject: Re: new __strtol warning In-Reply-To: <20141113211039.883652C3B16@topped-with-meat.com> Message-ID: References: <20141113211039.883652C3B16@topped-with-meat.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 On Thu, 13 Nov 2014, Roland McGrath wrote: > Perhaps we should start adding -Werror=foo for certain warnings, and > -Werror=strict-prototypes is a fairly obvious safe candidate. Using plain -Werror seems better than enabling it for particular warnings only - we just need an appropriate policy on selective disabling of warnings. > 2. Change the definition to be a prototype. This is a transformation we > have been generically doing, so it doesn't hurt. I'm not sure what our > current scheme is for ensuring there is a prior declaration for every > non-static function when the definition uses a prototype so that > -Wstrict-prototypes doesn't catch it. Nothing we have more automated > than human eyeballs will catch useless extra symbol names this way. I've committed this patch that fixes those warnings by using a prototype definition for __strtol. Tested for x86_64 that stripped installed shared libraries are unchanged by this patch. > 3. Rework your change so that it does not use the name __strtol at all, > while still using __strtoull et al. This is the ideal solution for this > case, because strtol is a C89 function and so __strtol will never be > used anywhere. It's always nice to clean the symbol table of useless > symbols and have the DWARF data use the canonical name for a function. That seemed an excessive complication (as the extra name is harmless and simplifies the code). 2014-11-13 Joseph Myers * stdlib/strtol.c (__strtol): Use prototype definition. diff --git a/stdlib/strtol.c b/stdlib/strtol.c index bd59180..60a33c0 100644 --- a/stdlib/strtol.c +++ b/stdlib/strtol.c @@ -104,10 +104,7 @@ libc_hidden_def (INTERNAL (strtol)) INT -__strtol (nptr, endptr, base) - const STRING_TYPE *nptr; - STRING_TYPE **endptr; - int base; +__strtol (const STRING_TYPE *nptr, STRING_TYPE **endptr, int base) { return INTERNAL (__strtol_l) (nptr, endptr, base, 0, _NL_CURRENT_LOCALE); }