[RFA] win32-nat printf and sprintf removal

[RFA] win32-nat printf and sprintf removal

[Pierre Muller]

I replaced printf by printf_unfiltered (or printf_filtered 
when there where other printf_filtered in the same function)
and sprintf by xasprintf in win32-nat.c source.

Does this enter in the obvious fix rules or not?

2002-02-08  Pierre Muller  <muller@ics.u-strasbg.fr>

	* win32-nat.c : Remove use of printf and sprintf functions.


Index: win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.47
diff -u -p -r1.47 win32-nat.c
--- win32-nat.c	2002/02/06 09:27:29	1.47
+++ win32-nat.c	2002/02/08 17:24:56
@@ -86,10 +86,10 @@ static int debug_registers_used = 0;
  #define CYGWIN_SIGNAL_STRING "cygwin: signal"
  
  #define CHECK(x)	check (x, __FILE__,__LINE__)
-#define DEBUG_EXEC(x)	if (debug_exec)		printf x
-#define DEBUG_EVENTS(x)	if (debug_events)	printf x
-#define DEBUG_MEM(x)	if (debug_memory)	printf x
-#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf x
+#define DEBUG_EXEC(x)	if (debug_exec)		printf_unfiltered x 
+#define DEBUG_EVENTS(x)	if (debug_events)	printf_unfiltered x
+#define DEBUG_MEM(x)	if (debug_memory)	printf_unfiltered x
+#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf_unfiltered x
  
  /* Forward declaration */
  extern struct target_ops child_ops;
@@ -785,7 +785,7 @@ info_dll_command (char *ignore, int from
    if (!so->next)
      return;
  
-  printf ("%*s  Load Address\n", -max_dll_name_len, "DLL Name");
+  printf_filtered ("%*s  Load Address\n", -max_dll_name_len, "DLL Name"); 
    while ((so = so->next) != NULL)
      printf_filtered ("%*s  %08lx\n", -max_dll_name_len, so->name, so->load_addr);
  
@@ -826,7 +826,7 @@ handle_output_debug_string (struct targe
  }
  
  #define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
-  printf ("gdb: Target exception %s at 0x%08lx\n", x, \
+  printf_unfiltered ("gdb: Target exception %s at 0x%08lx\n", x, \
    (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionAddress)
  
  static int
@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
    int pid = PIDGET (ptid);
  
    if ((DWORD) pid == current_event.dwProcessId)
-    sprintf (buf, "process %d", pid);
+    xaprintf (buf, "process %d", pid);
    else
-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
    return buf;
  }
  
@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
  	{
  	  int len = strlen (oldini);
  	  char *newini = alloca (len + 1);
-	  sprintf (newini, "%.*s.gdbinit", 
+	  xasprintf (newini, "%.*s.gdbinit", 
  	    (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
  	  warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
  	}



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

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

[Andrew Cagney]

> I replaced printf by printf_unfiltered (or printf_filtered 
> when there where other printf_filtered in the same function)
> and sprintf by xasprintf in win32-nat.c source.
> 
> Does this enter in the obvious fix rules or not?


Yes!!!!  You also earn huge numbers of brownie points from the Insight 
and other GUI developers.

Thanks!
Andrew


> 2002-02-08  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> * win32-nat.c : Remove use of printf and sprintf functions.
> 

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

[muller]

At 14:00 08/02/02 -0500, Andrew Cagney wrote:
>> I replaced printf by printf_unfiltered (or printf_filtered 
>> when there where other printf_filtered in the same function)
>> and sprintf by xasprintf in win32-nat.c source.
>> 
>> Does this enter in the obvious fix rules or not?
>
>
>Yes!!!!  You also earn huge numbers of brownie points from the Insight 
>and other GUI developers.

Done.

Don't forget that I also use GDB integrated in my
IDE (for Free Pascal compiler), thus this change
is also good for me.

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

[Christopher Faylor]

On Fri, Feb 08, 2002 at 06:29:11PM +0100, Pierre Muller wrote:
>
>I replaced printf by printf_unfiltered (or printf_filtered 
>when there where other printf_filtered in the same function)
>and sprintf by xasprintf in win32-nat.c source.

Since I wasn't aware of xasprintf, I checked it out.  It takes
a different first argument than sprintf.  xasprintf takes a char **.
sprintf takes a char *.

Didn't you notice an increased number of warnings with this patch?
I don't think you changed these calls correctly.

cgf

>Does this enter in the obvious fix rules or not?
>
>2002-02-08  Pierre Muller  <muller@ics.u-strasbg.fr>
>
>	* win32-nat.c : Remove use of printf and sprintf functions.
>
>
>Index: win32-nat.c
>===================================================================
>RCS file: /cvs/src/src/gdb/win32-nat.c,v
>retrieving revision 1.47
>diff -u -p -r1.47 win32-nat.c
>--- win32-nat.c	2002/02/06 09:27:29	1.47
>+++ win32-nat.c	2002/02/08 17:24:56
>@@ -86,10 +86,10 @@ static int debug_registers_used = 0;
>  #define CYGWIN_SIGNAL_STRING "cygwin: signal"
>  
>  #define CHECK(x)	check (x, __FILE__,__LINE__)
>-#define DEBUG_EXEC(x)	if (debug_exec)		printf x
>-#define DEBUG_EVENTS(x)	if (debug_events)	printf x
>-#define DEBUG_MEM(x)	if (debug_memory)	printf x
>-#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf x
>+#define DEBUG_EXEC(x)	if (debug_exec)		printf_unfiltered x 
>+#define DEBUG_EVENTS(x)	if (debug_events)	printf_unfiltered x
>+#define DEBUG_MEM(x)	if (debug_memory)	printf_unfiltered x
>+#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf_unfiltered x
>  
>  /* Forward declaration */
>  extern struct target_ops child_ops;
>@@ -785,7 +785,7 @@ info_dll_command (char *ignore, int from
>    if (!so->next)
>      return;
>  
>-  printf ("%*s  Load Address\n", -max_dll_name_len, "DLL Name");
>+  printf_filtered ("%*s  Load Address\n", -max_dll_name_len, "DLL Name"); 
>    while ((so = so->next) != NULL)
>      printf_filtered ("%*s  %08lx\n", -max_dll_name_len, so->name, so->load_addr);
>  
>@@ -826,7 +826,7 @@ handle_output_debug_string (struct targe
>  }
>  
>  #define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
>-  printf ("gdb: Target exception %s at 0x%08lx\n", x, \
>+  printf_unfiltered ("gdb: Target exception %s at 0x%08lx\n", x, \
>    (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionAddress)
>  
>  static int
>@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
>    int pid = PIDGET (ptid);
>  
>    if ((DWORD) pid == current_event.dwProcessId)
>-    sprintf (buf, "process %d", pid);
>+    xaprintf (buf, "process %d", pid);
>    else
>-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>    return buf;
>  }
>  
>@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
>  	{
>  	  int len = strlen (oldini);
>  	  char *newini = alloca (len + 1);
>-	  sprintf (newini, "%.*s.gdbinit", 
>+	  xasprintf (newini, "%.*s.gdbinit", 
>  	    (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
>  	  warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
>  	}
>
>
>
>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

-- 
Please do not send me personal email with cygwin questions.
Use the resources at http://cygwin.com/ .

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

[Martin M. Hunt]

Also, you must call free() after calls to xvasprintf(). 

-- 
Martin Hunt
GDB Engineer
Red Hat, Inc.

On Friday 08 February 2002 03:04 pm, Christopher Faylor wrote:
> On Fri, Feb 08, 2002 at 06:29:11PM +0100, Pierre Muller wrote:
> >I replaced printf by printf_unfiltered (or printf_filtered
> >when there where other printf_filtered in the same function)
> >and sprintf by xasprintf in win32-nat.c source.
>
> Since I wasn't aware of xasprintf, I checked it out.  It takes
> a different first argument than sprintf.  xasprintf takes a char **.
> sprintf takes a char *.
>
> Didn't you notice an increased number of warnings with this patch?
> I don't think you changed these calls correctly.
>
> cgf
>
> >Does this enter in the obvious fix rules or not?
> >
> >2002-02-08  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> >	* win32-nat.c : Remove use of printf and sprintf functions.
> >
> >
> >Index: win32-nat.c
> >===================================================================
> >RCS file: /cvs/src/src/gdb/win32-nat.c,v
> >retrieving revision 1.47
> >diff -u -p -r1.47 win32-nat.c
> >--- win32-nat.c	2002/02/06 09:27:29	1.47
> >+++ win32-nat.c	2002/02/08 17:24:56
> >@@ -86,10 +86,10 @@ static int debug_registers_used = 0;
> >  #define CYGWIN_SIGNAL_STRING "cygwin: signal"
> >
> >  #define CHECK(x)	check (x, __FILE__,__LINE__)
> >-#define DEBUG_EXEC(x)	if (debug_exec)		printf x
> >-#define DEBUG_EVENTS(x)	if (debug_events)	printf x
> >-#define DEBUG_MEM(x)	if (debug_memory)	printf x
> >-#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf x
> >+#define DEBUG_EXEC(x)	if (debug_exec)		printf_unfiltered x
> >+#define DEBUG_EVENTS(x)	if (debug_events)	printf_unfiltered x
> >+#define DEBUG_MEM(x)	if (debug_memory)	printf_unfiltered x
> >+#define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf_unfiltered x
> >
> >  /* Forward declaration */
> >  extern struct target_ops child_ops;
> >@@ -785,7 +785,7 @@ info_dll_command (char *ignore, int from
> >    if (!so->next)
> >      return;
> >
> >-  printf ("%*s  Load Address\n", -max_dll_name_len, "DLL Name");
> >+  printf_filtered ("%*s  Load Address\n", -max_dll_name_len, "DLL Name");
> >    while ((so = so->next) != NULL)
> >      printf_filtered ("%*s  %08lx\n", -max_dll_name_len, so->name,
> > so->load_addr);
> >
> >@@ -826,7 +826,7 @@ handle_output_debug_string (struct targe
> >  }
> >
> >  #define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
> >-  printf ("gdb: Target exception %s at 0x%08lx\n", x, \
> >+  printf_unfiltered ("gdb: Target exception %s at 0x%08lx\n", x, \
> >    (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionAddress)
> >
> >  static int
> >@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
> >    int pid = PIDGET (ptid);
> >
> >    if ((DWORD) pid == current_event.dwProcessId)
> >-    sprintf (buf, "process %d", pid);
> >+    xaprintf (buf, "process %d", pid);
> >    else
> >-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
> >+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
> >    return buf;
> >  }
> >
> >@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
> >  	{
> >  	  int len = strlen (oldini);
> >  	  char *newini = alloca (len + 1);
> >-	  sprintf (newini, "%.*s.gdbinit",
> >+	  xasprintf (newini, "%.*s.gdbinit",
> >  	    (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
> >  	  warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
> >  	}
> >
> >
> >
> >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

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

[Martin M. Hunt]

On Friday 08 February 2002 03:16 pm, Martin M. Hunt wrote:
> Also, you must call free() after calls to xvasprintf().
I meant xfree().

-- 
Martin Hunt
GDB Engineer
Red Hat, Inc.

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

[Pierre Muller]

At 00:04 09/02/2002 , Christopher Faylor a écrit:
>On Fri, Feb 08, 2002 at 06:29:11PM +0100, Pierre Muller wrote:
> >
> >I replaced printf by printf_unfiltered (or printf_filtered 
> >when there where other printf_filtered in the same function)
> >and sprintf by xasprintf in win32-nat.c source.
>
>Since I wasn't aware of xasprintf, I checked it out.  It takes
>a different first argument than sprintf.  xasprintf takes a char **.
>sprintf takes a char *.
>
>Didn't you notice an increased number of warnings with this patch?
>I don't think you changed these calls correctly.

You are right, I didn't check correctly :(

 >@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
> >    int pid = PIDGET (ptid);
> >  
> >    if ((DWORD) pid == current_event.dwProcessId)
> >-    sprintf (buf, "process %d", pid);
> >+    xaprintf (buf, "process %d", pid);
> >    else
> >-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
> >+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
> >    return buf;

As this is a static buffer, xasprintf can't be used here....
Andrew, why are the target_pid_to_str functions supposed to return static buffers?
Isn't that a big waste of memory?



> >@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
> >       {
> >         int len = strlen (oldini);
> >         char *newini = alloca (len + 1);
> >-        sprintf (newini, "%.*s.gdbinit", 
> >+        xasprintf (newini, "%.*s.gdbinit", 
> >           (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
> >         warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
> >       }

  I corrected this one to this patch,
which doesn't give any warning.
But the memory allocated for oldini is still lost....
Can I check this in?


Index: win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.50
diff -u -p -r1.50 win32-nat.c
--- win32-nat.c 2002/02/08 23:12:16     1.50
+++ win32-nat.c 2002/02/14 11:16:15
@@ -2008,10 +2008,11 @@ _initialize_check_for_gdb_ini (void)
        if (access (oldini, 0) == 0)
         {
           int len = strlen (oldini);
-         char *newini = alloca (len + 1);
-         sprintf (newini, "%.*s.gdbinit",
+         char *newini;
+         xasprintf (&newini, "%.*s.gdbinit",
             (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
           warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
+         xfree (newini);
         }
      }
  }





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

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

[Pierre Muller]

> > >@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
> > >       {
> > >         int len = strlen (oldini);
> > >         char *newini = alloca (len + 1);
> > >-        sprintf (newini, "%.*s.gdbinit", 
> > >+        xasprintf (newini, "%.*s.gdbinit", 
> > >           (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
> > >         warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
> > >       }
>
>   I corrected this one to this patch,
>which doesn't give any warning.
>But the memory allocated for oldini is still lost....

Whoops, once again, I clearly show my lack of C knowledge....
oldini is set by using alloca, and I just discovered that alloca
function does automatic disposal of the memory allocated at function exit.
  
  



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

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

[Andrew Cagney]

>>@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
> 
>> > int pid = PIDGET (ptid);
>> > > > if ((DWORD) pid == current_event.dwProcessId)
>> >-    sprintf (buf, "process %d", pid);
>> >+    xaprintf (buf, "process %d", pid);
>> > else
>> >-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>> >+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>> > return buf;
> 
> 
> As this is a static buffer, xasprintf can't be used here....
> Andrew, why are the target_pid_to_str functions supposed to return static buffers?
> Isn't that a big waste of memory?

Different coding styles.  People use static buffers (making the code 
non-reentrant) and sprintf() (making the code prone to buffer overruns) 
for a number of reasons.  One is that the programmer does know the 
lenght of the buffer and does know it won't be called re-entrantly so, 
rather than contend with cleanups, they use a static buffer.

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).

Andrew

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

[Christopher Faylor]

On Thu, Feb 14, 2002 at 12:17:17PM +0100, Pierre Muller wrote:
> >@@ -1763,9 +1763,9 @@ cygwin_pid_to_str (ptid_t ptid)
>> >    int pid = PIDGET (ptid);
>> >  
>> >    if ((DWORD) pid == current_event.dwProcessId)
>> >-    sprintf (buf, "process %d", pid);
>> >+    xaprintf (buf, "process %d", pid);
>> >    else
>> >-    sprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>> >+    xasprintf (buf, "thread %ld.0x%x", current_event.dwProcessId, pid);
>> >    return buf;
>
>As this is a static buffer, xasprintf can't be used here....
>Andrew, why are the target_pid_to_str functions supposed to return static buffers?
>Isn't that a big waste of memory?

No.

>> >@@ -2009,7 +2009,7 @@ _initialize_check_for_gdb_ini (void)
>> >       {
>> >         int len = strlen (oldini);
>> >         char *newini = alloca (len + 1);
>> >-        sprintf (newini, "%.*s.gdbinit", 
>> >+        xasprintf (newini, "%.*s.gdbinit", 
>> >           (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
>> >         warning ("obsolete '%s' found. Rename to '%s'.", oldini, newini);
>> >       }
>
>  I corrected this one to this patch,
>which doesn't give any warning.
>But the memory allocated for oldini is still lost....
>Can I check this in?

No.  Please don't mess with the sprintfs in win32-nat.c.

cgf

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

[Christopher Faylor]

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

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

[Pierre Muller]

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

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

[Christopher Faylor]

On Thu, Feb 14, 2002 at 05:44:10PM +0100, Pierre Muller wrote:
>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 :()

No permanent harm done.  I *really* appreciate all the work that you're
doing on win32-nat.c.  You're adding features that I've been dreaming about
for years.

Which reminds me.  I have a patch pending that I really should apply.
It allows use of ">" "<" constructs from the command line.

Where did I put that patch?  Rustle, rustle...

cgf