From: Pierre Muller <muller@cerbere.u-strasbg.fr>
To: gdb-patches@sources.redhat.com
Subject: Re: [RFA] win32-nat printf and sprintf removal
Date: Thu, 14 Feb 2002 08:44:00 -0000 [thread overview]
Message-ID: <4.2.0.58.20020214173516.01d42828@ics.u-strasbg.fr> (raw)
In-Reply-To: <20020214161350.GM23253@redhat.com>
At 17:13 14/02/2002 , Christopher Faylor a écrit:
>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.
You are completely right, I need to be more cautious,
especially as my C knowledge still is quite lacunar.
(I didn't know about the automatic disposal for alloca until today :()
>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.
But there are also quite a lot of printf still lying around in gdb sources.
I simply now believe that indeed
changing printf into printf_unfiltered and
even changing fprintf (stderr, ...) into fprintf_unfiltered (gdb_stderr,...)
are much more "obvious" than the sprintf to xasprintf one.
Moreover only the two former changes make improovements for GUI or
other porgrams using GDB internally.
Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07 Fax : (33)-3-88-41-40-99
next prev parent reply other threads:[~2002-02-14 16:44 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-02-08 9:29 Pierre Muller
2002-02-08 11:00 ` Andrew Cagney
2002-02-08 11:34 ` muller
2002-02-08 15:04 ` Christopher Faylor
2002-02-08 15:17 ` Martin M. Hunt
2002-02-08 15:48 ` Martin M. Hunt
2002-02-14 3:17 ` Pierre Muller
2002-02-14 3:36 ` Pierre Muller
2002-02-14 7:31 ` Andrew Cagney
2002-02-14 8:13 ` Christopher Faylor
2002-02-14 8:44 ` Pierre Muller [this message]
2002-02-14 8:49 ` Christopher Faylor
2002-02-14 7:59 ` Christopher Faylor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4.2.0.58.20020214173516.01d42828@ics.u-strasbg.fr \
--to=muller@cerbere.u-strasbg.fr \
--cc=gdb-patches@sources.redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox