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

Pedro Alves
2016-06-29T10:26:15Z

(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