This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] win32-nat printf and sprintf removal


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]