Re: [PATCH] constify to_attach

[Pedro Alves <palves@redhat.com>]

On 05/21/2014 07:20 PM, Tom Tromey wrote:

> The code in parse_pid_to_attach seems a little bogus to me.  If there
> is a platform with a broken strtoul, we have better methods for fixing
> the issue now.  However, I left the code as is since it is clearly ok
> to do so.

Yeah.  We should probably use get_number or some such that accepts
convenience vars even.

> +parse_pid_to_attach (const char *args)
>  {
>    unsigned long pid;
>    char *dummy;
> @@ -3257,7 +3257,7 @@ parse_pid_to_attach (char *args)
>    if (!args)
>      error_no_arg (_("process-id to attach"));
>  
> -  dummy = args;
> +  dummy = (char *) args;
>    pid = strtoul (args, &dummy, 0);
>    /* Some targets don't set errno on errors, grrr!  */
>    if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)])

errno would be necessary to catch overflow, but not to check whether
the number was syntactically correct.  strtoul always sets *endptr to
point to the address of the first invalid character (and never to NULL).

So you could just remove the 'dummy' assignment.

But I'll understand if you want to keep it.

The patch looks fine to me.

-- 
Pedro Alves