RFA: remote.c: parse thread ID's as unsigned values

RFA: remote.c: parse thread ID's as unsigned values

[Jim Blandy]

The GDB manual isn't clear on whether thread ID's carried in the
responses to the qfThreadInfo and the qC packet may have a sign; I'm
assuming that they're just a series of hex digits.

2004-10-27  Jim Blandy  <jimb@redhat.com>

	* remote.c (remote_threads_info, remote_current_thread): Use
	strtoul to parse thread ID numbers.

Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.134
diff -c -r1.134 remote.c
*** gdb/remote.c	26 Apr 2004 09:02:41 -0000	1.134
--- gdb/remote.c	27 Oct 2004 21:26:40 -0000
***************
*** 1755,1761 ****
    putpkt ("qC");
    getpkt (buf, (rs->remote_packet_size), 0);
    if (buf[0] == 'Q' && buf[1] == 'C')
!     return pid_to_ptid (strtol (&buf[2], NULL, 16));
    else
      return oldpid;
  }
--- 1755,1766 ----
    putpkt ("qC");
    getpkt (buf, (rs->remote_packet_size), 0);
    if (buf[0] == 'Q' && buf[1] == 'C')
!     /* Use strtoul here, so we'll correctly parse values whose highest
!        bit is set.  The protocol carries them as a simple series of
!        hex digits; in the absence of a sign, strtol will see such
!        values as positive numbers out of range for signed 'long', and
!        return LONG_MAX to indicate an overflow.  */
!     return pid_to_ptid (strtoul (&buf[2], NULL, 16));
    else
      return oldpid;
  }
***************
*** 1802,1808 ****
  	    {
  	      do
  		{
! 		  tid = strtol (bufp, &bufp, 16);
  		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
  		    add_thread (pid_to_ptid (tid));
  		}
--- 1807,1819 ----
  	    {
  	      do
  		{
! 		  /* Use strtoul here, so we'll correctly parse values
! 		     whose highest bit is set.  The protocol carries
! 		     them as a simple series of hex digits; in the
! 		     absence of a sign, strtol will see such values as
! 		     positive numbers out of range for signed 'long',
! 		     and return LONG_MAX to indicate an overflow.  */
! 		  tid = strtoul (bufp, &bufp, 16);
  		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
  		    add_thread (pid_to_ptid (tid));
  		}

Re: RFA: remote.c: parse thread ID's as unsigned values

[Jim Blandy]

One month ping.

Jim Blandy <jimb@redhat.com> writes:

> The GDB manual isn't clear on whether thread ID's carried in the
> responses to the qfThreadInfo and the qC packet may have a sign; I'm
> assuming that they're just a series of hex digits.
> 
> 2004-10-27  Jim Blandy  <jimb@redhat.com>
> 
> 	* remote.c (remote_threads_info, remote_current_thread): Use
> 	strtoul to parse thread ID numbers.
> 
> Index: gdb/remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.134
> diff -c -r1.134 remote.c
> *** gdb/remote.c	26 Apr 2004 09:02:41 -0000	1.134
> --- gdb/remote.c	27 Oct 2004 21:26:40 -0000
> ***************
> *** 1755,1761 ****
>     putpkt ("qC");
>     getpkt (buf, (rs->remote_packet_size), 0);
>     if (buf[0] == 'Q' && buf[1] == 'C')
> !     return pid_to_ptid (strtol (&buf[2], NULL, 16));
>     else
>       return oldpid;
>   }
> --- 1755,1766 ----
>     putpkt ("qC");
>     getpkt (buf, (rs->remote_packet_size), 0);
>     if (buf[0] == 'Q' && buf[1] == 'C')
> !     /* Use strtoul here, so we'll correctly parse values whose highest
> !        bit is set.  The protocol carries them as a simple series of
> !        hex digits; in the absence of a sign, strtol will see such
> !        values as positive numbers out of range for signed 'long', and
> !        return LONG_MAX to indicate an overflow.  */
> !     return pid_to_ptid (strtoul (&buf[2], NULL, 16));
>     else
>       return oldpid;
>   }
> ***************
> *** 1802,1808 ****
>   	    {
>   	      do
>   		{
> ! 		  tid = strtol (bufp, &bufp, 16);
>   		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
>   		    add_thread (pid_to_ptid (tid));
>   		}
> --- 1807,1819 ----
>   	    {
>   	      do
>   		{
> ! 		  /* Use strtoul here, so we'll correctly parse values
> ! 		     whose highest bit is set.  The protocol carries
> ! 		     them as a simple series of hex digits; in the
> ! 		     absence of a sign, strtol will see such values as
> ! 		     positive numbers out of range for signed 'long',
> ! 		     and return LONG_MAX to indicate an overflow.  */
> ! 		  tid = strtoul (bufp, &bufp, 16);
>   		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
>   		    add_thread (pid_to_ptid (tid));
>   		}

Re: RFA: remote.c: parse thread ID's as unsigned values

[Michael Snyder]

Jim Blandy wrote:
> One month ping.

Well heck, I'm not the remote.c maintainer, but I'm the thread
maintainer (or one of them).

Your change certainly seems reasonable to me.  Can't think why
we would want thread ids to be sign extended...

Under the assumption that you've run into a situation where this
is applicable, and the change tests out well, I'll give it the nod.

Michael

> 
> Jim Blandy <jimb@redhat.com> writes:
> 
> 
>>The GDB manual isn't clear on whether thread ID's carried in the
>>responses to the qfThreadInfo and the qC packet may have a sign; I'm
>>assuming that they're just a series of hex digits.
>>
>>2004-10-27  Jim Blandy  <jimb@redhat.com>
>>
>>	* remote.c (remote_threads_info, remote_current_thread): Use
>>	strtoul to parse thread ID numbers.
>>
>>Index: gdb/remote.c
>>===================================================================
>>RCS file: /cvs/src/src/gdb/remote.c,v
>>retrieving revision 1.134
>>diff -c -r1.134 remote.c
>>*** gdb/remote.c	26 Apr 2004 09:02:41 -0000	1.134
>>--- gdb/remote.c	27 Oct 2004 21:26:40 -0000
>>***************
>>*** 1755,1761 ****
>>    putpkt ("qC");
>>    getpkt (buf, (rs->remote_packet_size), 0);
>>    if (buf[0] == 'Q' && buf[1] == 'C')
>>!     return pid_to_ptid (strtol (&buf[2], NULL, 16));
>>    else
>>      return oldpid;
>>  }
>>--- 1755,1766 ----
>>    putpkt ("qC");
>>    getpkt (buf, (rs->remote_packet_size), 0);
>>    if (buf[0] == 'Q' && buf[1] == 'C')
>>!     /* Use strtoul here, so we'll correctly parse values whose highest
>>!        bit is set.  The protocol carries them as a simple series of
>>!        hex digits; in the absence of a sign, strtol will see such
>>!        values as positive numbers out of range for signed 'long', and
>>!        return LONG_MAX to indicate an overflow.  */
>>!     return pid_to_ptid (strtoul (&buf[2], NULL, 16));
>>    else
>>      return oldpid;
>>  }
>>***************
>>*** 1802,1808 ****
>>  	    {
>>  	      do
>>  		{
>>! 		  tid = strtol (bufp, &bufp, 16);
>>  		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
>>  		    add_thread (pid_to_ptid (tid));
>>  		}
>>--- 1807,1819 ----
>>  	    {
>>  	      do
>>  		{
>>! 		  /* Use strtoul here, so we'll correctly parse values
>>! 		     whose highest bit is set.  The protocol carries
>>! 		     them as a simple series of hex digits; in the
>>! 		     absence of a sign, strtol will see such values as
>>! 		     positive numbers out of range for signed 'long',
>>! 		     and return LONG_MAX to indicate an overflow.  */
>>! 		  tid = strtoul (bufp, &bufp, 16);
>>  		  if (tid != 0 && !in_thread_list (pid_to_ptid (tid)))
>>  		    add_thread (pid_to_ptid (tid));
>>  		}
> 
> 
> 

Re: RFA: remote.c: parse thread ID's as unsigned values

[Daniel Jacobowitz]

On Fri, Dec 03, 2004 at 01:48:12PM -0800, Michael Snyder wrote:
> Jim Blandy wrote:
> >One month ping.
> 
> Well heck, I'm not the remote.c maintainer, but I'm the thread
> maintainer (or one of them).
> 
> Your change certainly seems reasonable to me.  Can't think why
> we would want thread ids to be sign extended...
> 
> Under the assumption that you've run into a situation where this
> is applicable, and the change tests out well, I'll give it the nod.

Fine by me too - but if the manual is unclear, Jim, could you clarify
it?

-- 
Daniel Jacobowitz

Re: RFA: remote.c: parse thread ID's as unsigned values

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Fri, Dec 03, 2004 at 01:48:12PM -0800, Michael Snyder wrote:
> > Jim Blandy wrote:
> > >One month ping.
> > 
> > Well heck, I'm not the remote.c maintainer, but I'm the thread
> > maintainer (or one of them).
> > 
> > Your change certainly seems reasonable to me.  Can't think why
> > we would want thread ids to be sign extended...
> > 
> > Under the assumption that you've run into a situation where this
> > is applicable, and the change tests out well, I'll give it the nod.
> 
> Fine by me too - but if the manual is unclear, Jim, could you clarify
> it?

I've committed the patch, thanks.  How's this for the documentation
change?

2004-12-07  Jim Blandy  <jimb@redhat.com>

	* gdb.texinfo (General Query Packets): Specify that thread ID's in
	the 'qC' and 'qThreadInfo' packets are unsigned hexidecimal
	numbers.

Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.228
diff -c -p -r1.228 gdb.texinfo
*** gdb/doc/gdb.texinfo	7 Dec 2004 11:06:04 -0000	1.228
--- gdb/doc/gdb.texinfo	7 Dec 2004 20:14:33 -0000
*************** Return the current thread id.
*** 20834,20840 ****
  Reply:
  @table @samp
  @item @code{QC}@var{pid}
! Where @var{pid} is a HEX encoded 16 bit process id.
  @item *
  Any other reply implies the old pid.
  @end table
--- 20834,20840 ----
  Reply:
  @table @samp
  @item @code{QC}@var{pid}
! Where @var{pid} is an unsigned hexidecimal process id.
  @item *
  Any other reply implies the old pid.
  @end table
*************** a comma-separated list of thread ids
*** 20863,20872 ****
  @end table
  
  In response to each query, the target will reply with a list of one or
! more thread ids, in big-endian hex, separated by commas.  @value{GDBN}
! will respond to each reply with a request for more thread ids (using the
! @code{qs} form of the query), until the target responds with @code{l}
! (lower-case el, for @code{'last'}).
  
  @item @code{q}@code{ThreadExtraInfo}@code{,}@var{id} --- extra thread info
  
--- 20863,20872 ----
  @end table
  
  In response to each query, the target will reply with a list of one or
! more thread ids, in big-endian unsigned hex, separated by commas.
! @value{GDBN} will respond to each reply with a request for more thread
! ids (using the @code{qs} form of the query), until the target responds
! with @code{l} (lower-case el, for @code{'last'}).
  
  @item @code{q}@code{ThreadExtraInfo}@code{,}@var{id} --- extra thread info
  

Re: RFA: remote.c: parse thread ID's as unsigned values

[Eli Zaretskii]

> Cc: Michael Snyder <msnyder@redhat.com>, gdb-patches@sources.redhat.com
> From: Jim Blandy <jimb@redhat.com>
> Date: 07 Dec 2004 15:20:54 -0500
> 
> How's this for the documentation change?

Fine, thanks.

Re: RFA: remote.c: parse thread ID's as unsigned values

[Jim Blandy]

"Eli Zaretskii" <eliz@gnu.org> writes:

> > Cc: Michael Snyder <msnyder@redhat.com>, gdb-patches@sources.redhat.com
> > From: Jim Blandy <jimb@redhat.com>
> > Date: 07 Dec 2004 15:20:54 -0500
> > 
> > How's this for the documentation change?
> 
> Fine, thanks.

Committed --- thanks.