From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22317 invoked by alias); 14 Feb 2002 16:13:46 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 22270 invoked from network); 14 Feb 2002 16:13:44 -0000 Received: from unknown (HELO lacrosse.corp.redhat.com) (12.107.208.154) by sources.redhat.com with SMTP; 14 Feb 2002 16:13:44 -0000 Received: from cgf.cipe.redhat.com (dhcpd80.meridian.redhat.com [172.16.47.80]) by lacrosse.corp.redhat.com (8.11.6/8.9.3) with ESMTP id g1EGDhd02570 for ; Thu, 14 Feb 2002 11:13:44 -0500 Received: (from cgf@localhost) by cgf.cipe.redhat.com (8.11.6/8.8.7) id g1EGDoF30739 for gdb-patches@sources.redhat.com; Thu, 14 Feb 2002 11:13:50 -0500 Date: Thu, 14 Feb 2002 08:13:00 -0000 From: Christopher Faylor To: gdb-patches@sources.redhat.com Subject: Re: [RFA] win32-nat printf and sprintf removal Message-ID: <20020214161350.GM23253@redhat.com> Reply-To: gdb-patches@sources.redhat.com Mail-Followup-To: gdb-patches@sources.redhat.com References: <4.2.0.58.20020208182442.00ad05e0@ics.u-strasbg.fr> <4.2.0.58.20020208182442.00ad05e0@ics.u-strasbg.fr> <4.2.0.58.20020214121240.01a80208@ics.u-strasbg.fr> <3C6BD855.8030003@cygnus.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3C6BD855.8030003@cygnus.com> User-Agent: Mutt/1.3.23.1i X-SW-Source: 2002-02/txt/msg00411.txt.bz2 On Thu, Feb 14, 2002 at 10:31:33AM -0500, Andrew Cagney wrote: >Suggest adding a comment just above each sprintf() call indicating that >buf is static (at least that way the next person won't be puzzled by >this). There are three sprintfs in win32-nat.c. One uses a static buffer of 80 bytes (which is overkill). The 'static char buf[80]' is two or three lines above the use of sprintf. The other use of sprintf uses an alloca'ed buffer. The alloca is directly above the sprintf. I don't think it makes sense to mention "this buffer is static" one line below the definition of the buffer or "this buffer is allocated from the stack" directly after the buffer is allocated on the stack. The moral of the story here is not that more comments are needed (at least not in this case). The true moral is that you should be sensitive to warnings in the code, you should be *very* sensitive to an increase in warnings (in this case from zero to three) and you should test changes thoroughly before submitting an "obvious" fix. It's possible, I guess, that the change of printf to printif_unfiltered was an obvious fix. The change from sprintf to xasprintf was not. You just have to do a 'grep -w sprintf' on the gdb sources to see that sprintf is used quite frequently, so any change to xasprintf would have to be justified. cgf