Re: [PATCH 3/3, doc RFA] gdbserver debug_printf+timestamps: main patch

[Pedro Alves <palves@redhat.com>]

On 01/16/2014 11:37 PM, Doug Evans wrote:
> +++ b/gdb/gdbserver/debug.c
> @@ -0,0 +1,86 @@
> +/* General utility routines for the remote server for GDB.

These ...

> +++ b/gdb/gdbserver/debug.h
> @@ -0,0 +1,48 @@
> +/* General utility routines for the remote server for GDB.

... need updating.

> +#endif /* UTILS_H */

This too -- DEBUG_H.

> +debug_printf (const char *msg, ...)
> +{
> +  va_list args;
> +#ifdef HAVE_GETTIMEOFDAY
> +  static int new_line = 1;
> +

Note this isn't thread safe either.  Maybe add a comment
in case we ever try to make timespamping work in the IPA?

> +  options = delim_string_to_char_ptr_vec (arg + sizeof ("--debug=") - 1, ',');
> +
> +  for (ix = 0; VEC_iterate (char_ptr, options, ix, option); ++ix)
> +    {

I don't see much point in this extra copying over plain old strtok
(already used in the --disable-packet= code), but OK...

>        else if (strcmp (*next_arg, "--debug") == 0)
> -	debug_threads = 1;
> +	parse_debug_options (*next_arg);
> +      else if (strncmp (*next_arg, "--debug=", sizeof ("--debug=") - 1) == 0)
> +	{
> +	  if (parse_debug_options (*next_arg) != 0)
> +	    exit (1);
> +	}

This should be hooked in "monitor set debug ..." as well.

Otherwise looks good to me.

Thanks,
-- 
Pedro Alves