This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/3, doc RFA] gdbserver debug_printf+timestamps: main patch
- From: Pedro Alves <palves at redhat dot com>
- To: Doug Evans <dje at google dot com>
- Cc: Yao Qi <yao at codesourcery dot com>, eliz at gnu dot org, gdb-patches <gdb-patches at sourceware dot org>
- Date: Mon, 20 Jan 2014 16:14:08 +0000
- Subject: Re: [PATCH 3/3, doc RFA] gdbserver debug_printf+timestamps: main patch
- Authentication-results: sourceware.org; auth=none
- References: <yjt2zjnztait dot fsf at ruffy dot mtv dot corp dot google dot com> <52B1842F dot 5020401 at redhat dot com> <21205 dot 55987 dot 69477 dot 892571 at ruffy dot mtv dot corp dot google dot com> <52D81569 dot 3080006 at redhat dot com> <CADPb22SK=YQeOcOdNPXERxXKOT2E64k=pHhw0GBiLj4LQhL-Ag at mail dot gmail dot com> <52D82AD5 dot 7000306 at redhat dot com> <21208 dot 27400 dot 695984 dot 88504 at ruffy dot mtv dot corp dot google dot com> <21208 dot 27945 dot 781450 dot 905336 at ruffy dot mtv dot corp dot google dot com> <52D92610 dot 4010202 at redhat dot com> <21209 dot 45676 dot 594470 dot 76921 at ruffy dot mtv dot corp dot google dot 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