Re: [PATCH 1/2] gdbserver, read_ptid: handle '0' and '-1' thread ids

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello Markus,

Markus Metzger <markus.t.metzger@intel.com> writes:

> The special thread id '-1' means 'all threads'.
> The special thread id '0' means 'any thread'.
>
> Read_ptid () currently returns
>
>     <current pid>.-1.0
>
> and
>
>     <current pid>.0.0
>
> respectively.
>
> Change that to minus_one_ptid for '-1' and to null_ptid for '0'.
>
> CC: Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
> ---
>  gdbserver/remote-utils.cc | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

Nice improvement. Just a minor comment.

> diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
> index 15f073dd6be..9562cc2e1fb 100644
> --- a/gdbserver/remote-utils.cc
> +++ b/gdbserver/remote-utils.cc
> @@ -566,6 +566,26 @@ read_ptid (const char *buf, const char **obuf)
>    const char *p = buf;
>    const char *pp;
>  
> +  /* Handle special thread ids.  */
> +  if (*p == '0')

I think this should also check the character after '0':

-  if (*p == '0')
+  if (*p == '0' && !isxdigit (p[1]))

> +    {
> +      if (obuf)
> +	*obuf = p + 1;
> +
> +      return null_ptid;
> +    }
> +
> +  if (*p == '-')
> +    {
> +      if (p[1] != '1')

Same here:

-      if (p[1] != '1')
+      if (p[1] != '1' || isxdigit (p[2]))

> +	return null_ptid;
> +
> +      if (obuf)
> +	*obuf = p + 2;
> +
> +      return minus_one_ptid;
> +    }
> +
>    if (*p == 'p')
>      {
>        ULONGEST pid;

From reading the RSP documentation, thread ids are followed only by ',',
';', ':' or '\0', but checking for an hex digit should be enough.

With the changes:

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

-- 
Thiago