Re: [PATCH] Use strtok_r instead of strsep in rust_get_disr_info

[Manish Goregaokar <manish@mozilla.com>]

Fixed.

From 6b577bb78279a453c77b08c5a1090d8dec1eeee7 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manish@mozilla.com>
Date: Wed, 29 Jun 2016 15:42:28 +0530
Subject: [PATCH] Use strtok_r instead of strsep in rust_get_disr_info

strsep doesn't exist on Windows.

2016-06-29  Manish Goregaokar  <manish@mozilla.com>

gdb/ChangeLog:
    * rust-lang.c (rust_get_disr_info): Use strtok_r instead of strsep.
---
 gdb/rust-lang.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 23ddd5a..5910d4b 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -121,7 +121,7 @@ rust_get_disr_info (struct type *type, const
gdb_byte *valaddr,
   if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
            strlen (RUST_ENUM_PREFIX)) == 0)
     {
-      char *name, *tail, *token;
+      char *tail, *token, *name, *saveptr;
       unsigned long fieldno;
       struct type *member_type;
       LONGEST value;
@@ -145,7 +145,9 @@ rust_get_disr_info (struct type *type, const
gdb_byte *valaddr,
          traversed in order to find the field (which may be several
fields deep)
          and the variantname is the name of the variant of the case when the
          field is zero.  */
-      while ((token = strsep (&tail, "$")) != NULL)
+      for (token = strtok_r (tail, "$", &saveptr);
+           token != NULL;
+           token = strtok_r (NULL, "$", &saveptr))
         {
       if (sscanf (token, "%lu", &fieldno) != 1)
         {
@@ -161,8 +163,6 @@ rust_get_disr_info (struct type *type, const
gdb_byte *valaddr,
           member_type = TYPE_FIELD_TYPE (member_type, fieldno);
         }

-      if (token >= name + strlen (TYPE_FIELD_NAME (type, 0)))
-    error (_("Invalid form for %s"), RUST_ENUM_PREFIX);
       value = unpack_long (member_type, valaddr + embedded_offset);

       if (value == 0)
-- 
2.8.3

-Manish


On Wed, Jun 29, 2016 at 3:56 PM, Pedro Alves <palves@redhat.com> wrote:
> (stepping in since this fixes a build breakage.)
>
> On 06/29/2016 11:13 AM, Manish Goregaokar wrote:
>> strsep doesn't exist on windows
>
> "Windows", uppercase.
>
>>
>> 2016-06-29  Manish Goregaokar  <manish@mozilla.com>
>>
>> gdb/ChangeLog:
>>     * rust-lang.c (rust_get_disr_info): Use strtok_r instead of strsep
>
> Period at end of sentences.  Likewise on the commit log,
> while at it.
>
>> ---
>>  gdb/rust-lang.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
>> index 23ddd5a..961fdca 100644
>> --- a/gdb/rust-lang.c
>> +++ b/gdb/rust-lang.c
>> @@ -121,7 +121,7 @@ rust_get_disr_info (struct type *type, const
>> gdb_byte *valaddr,
>>    if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
>>             strlen (RUST_ENUM_PREFIX)) == 0)
>>      {
>> -      char *name, *tail, *token;
>> +      char *tail, *token, *name, *last = NULL;
>
> (I don't think you need to initialize this.  If you get
> a warning about it, it's likely revealing a bug.)
>
>>        unsigned long fieldno;
>>        struct type *member_type;
>>        LONGEST value;
>> @@ -138,6 +138,8 @@ rust_get_disr_info (struct type *type, const
>> gdb_byte *valaddr,
>>        cleanup = make_cleanup (xfree, name);
>>        tail = name + strlen (RUST_ENUM_PREFIX);
>>
>> +      token = strtok_r (tail, "$", &last);
>> +
>>        /* The location of the value that doubles as a discriminant is
>>           stored in the name of the field, as
>>           RUST$ENCODED$ENUM$<fieldno>$<fieldno>$...$<variantname>
>> @@ -145,7 +147,7 @@ rust_get_disr_info (struct type *type, const
>> gdb_byte *valaddr,
>>           traversed in order to find the field (which may be several
>> fields deep)
>>           and the variantname is the name of the variant of the case when the
>>           field is zero.  */
>> -      while ((token = strsep (&tail, "$")) != NULL)
>> +      do
>>          {
>>        if (sscanf (token, "%lu", &fieldno) != 1)
>
> We'll now reach here even if token is NULL the first time.
>
>>          {
>> @@ -159,10 +161,8 @@ rust_get_disr_info (struct type *type, const
>> gdb_byte *valaddr,
>>
>>            embedded_offset += TYPE_FIELD_BITPOS (member_type, fieldno) / 8;
>>            member_type = TYPE_FIELD_TYPE (member_type, fieldno);
>> -        }
>> +        } while ((token = strtok_r (NULL, "$", &last)) != NULL);
>
> IMO, this calls for a "for" loop instead of a do/while.  Like:
>
>  -      while ((token = strsep (&tail, "$")) != NULL)
>  +      for (token = strtok_r (tail, "$", &saveptr);
>  +           token != NULL;
>  +           token = strtok_r (NULL, "$", &saveptr))
>          {
>
> Thanks,
> Pedro Alves
>