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

Manish Goregaokar
2016-06-29T10:32:10Z

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
>