[RFA] Remove some sprintfs from vCont client support

[Daniel Jacobowitz <drow@mvista.com>]

On Thu, Oct 16, 2003 at 06:53:28PM -0400, Daniel Jacobowitz wrote:
> On Thu, Oct 16, 2003 at 06:27:07PM -0400, Andrew Cagney wrote:
> > >There are two probems:
> > >>
> > >>- the buffer can get very very large and that can blow the stack

If that concerns you then I suggest the alloca's in putpkt_binary,
which I found while fixing this.  That's a whole lot more worrisome,
since there's one as big as the packet plus one as big as the max
packet length.

> > >>- it isn't possible to audit this code (with out a deep understanding of 
> > >>that value) and hence demonstrate that the sprintf won't smash the 
> > >>stack/heap
> > >>
> > >>You'll need to also change the sprintf to snprintf (parameterized with 
> > >>remote_packet_size.
> > >
> > >
> > >I don't see a point in doing that until someone expresses interest in
> > >thread locking or some other feature which requires adding to the code. 
> > >The maximum length of any generated vcont packet is the length of:
> > >   vCont;C01:12341468;C02
> > >The minimum possible buffer size is about twenty times that.
> > 
> > I wrote "it isn't possible to audit this code (with out a deep 
> > understanding of that [remote_packet_size] value)".  The code should be 
> > locally robust.
> 
> I wouldn't call the minimum size a deep understanding.  It isn't
> documented anywhere in the code but I think that it should be.
> 
> But I'll fix it next week.

OK, so it wasn't the next week.  I took advantage of the new xstrprintf
function to get something I'm happy with.  How about you, Andrew - is
this patch OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-12-02  Daniel Jacobowitz  <drow@mvista.com>

	* remote.c (remote_vcont_resume): Use xstrprintf instead of sprintf.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.122
diff -u -p -r1.122 remote.c
--- remote.c	10 Nov 2003 21:20:44 -0000	1.122
+++ remote.c	2 Dec 2003 15:57:33 -0000
@@ -2578,7 +2578,7 @@ remote_vcont_resume (ptid_t ptid, int st
 {
   struct remote_state *rs = get_remote_state ();
   int pid = PIDGET (ptid);
-  char *buf = NULL;
+  char *buf = NULL, *outbuf;
   struct cleanup *old_cleanup;
 
   buf = xmalloc (rs->remote_packet_size);
@@ -2603,40 +2603,45 @@ remote_vcont_resume (ptid_t ptid, int st
 	 don't have any PID numbers the inferior will understand.  Make sure
 	 to only send forms that do not specify a PID.  */
       if (step && siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;S%02x", siggnal);
+	outbuf = xstrprintf ("vCont;S%02x", siggnal);
       else if (step)
-	sprintf (buf, "vCont;s");
+	outbuf = xstrprintf ("vCont;s");
       else if (siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;C%02x", siggnal);
+	outbuf = xstrprintf ("vCont;C%02x", siggnal);
       else
-	sprintf (buf, "vCont;c");
+	outbuf = xstrprintf ("vCont;c");
     }
   else if (pid == -1)
     {
       /* Resume all threads, with preference for INFERIOR_PTID.  */
       if (step && siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;S%02x:%x;c", siggnal, PIDGET (inferior_ptid));
+	outbuf = xstrprintf ("vCont;S%02x:%x;c", siggnal,
+			     PIDGET (inferior_ptid));
       else if (step)
-	sprintf (buf, "vCont;s:%x;c", PIDGET (inferior_ptid));
+	outbuf = xstrprintf ("vCont;s:%x;c", PIDGET (inferior_ptid));
       else if (siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;C%02x:%x;c", siggnal, PIDGET (inferior_ptid));
+	outbuf = xstrprintf ("vCont;C%02x:%x;c", siggnal,
+			     PIDGET (inferior_ptid));
       else
-	sprintf (buf, "vCont;c");
+	outbuf = xstrprintf ("vCont;c");
     }
   else
     {
       /* Scheduler locking; resume only PTID.  */
       if (step && siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;S%02x:%x", siggnal, pid);
+	outbuf = xstrprintf ("vCont;S%02x:%x", siggnal, pid);
       else if (step)
-	sprintf (buf, "vCont;s:%x", pid);
+	outbuf = xstrprintf ("vCont;s:%x", pid);
       else if (siggnal != TARGET_SIGNAL_0)
-	sprintf (buf, "vCont;C%02x:%x", siggnal, pid);
+	outbuf = xstrprintf ("vCont;C%02x:%x", siggnal, pid);
       else
-	sprintf (buf, "vCont;c:%x", pid);
+	outbuf = xstrprintf ("vCont;c:%x", pid);
     }
 
-  putpkt (buf);
+  gdb_assert (outbuf && strlen (outbuf) < rs->remote_packet_size);
+  make_cleanup (xfree, outbuf);
+
+  putpkt (outbuf);
 
   do_cleanups (old_cleanup);