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

[Pedro Alves <palves@redhat.com>]

On 01/17/2014 10:45 PM, Doug Evans wrote:
> Pedro Alves writes:
>  > 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.
> 
> I changed --debug= to --debug-format=, and added monitor commands.

Thanks.  I have no further comments.

-- 
Pedro Alves