This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Thu, Feb 17, 2011 at 16:15, Hui Zhu <teawater@gmail.com> wrote: > On Sat, Feb 12, 2011 at 02:45, Tom Tromey <tromey@redhat.com> wrote: >>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes: >> >>>> + ? ? ?gen_expr (expr, &pc, aexpr, &value); >>>> + >>>> + >>>> + ? ? ?if (value.optimized_out) >> >> There's no need to have 2 blank lines here. >> >> This function and some other new ones have no documentation. >> >>>> + ?{"printf", 0, 0, 0, 0}, ? /* 0x31 */ >> [...] >>>> + ? ?aop_printf = 0x31, >> >> If you add a new AX expression, you must update agentexpr.texi. >> >> I think any AX addition should also require a corresponding addition to >> gdbserver. >> >>>> +typedef void (printf_callback) (char *fbuf, char **expp, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct bp_location *loc, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct agent_expr *aexpr); >> >> From what I can see, the `loc' and `aexpr' arguments are passed through >> string_printf without interpretation. >> >> In a case like this it is customary to just add a single `void *data' >> argument and have the caller and callback agree on the type. ?Here, that >> type would be an instance of a struct wrapping the two values. >> >> This definition should not be here. >> >>>> ?static void >>>> +ui_printf (char *arg, struct ui_file *stream) >>>> +{ >>>> + ?string_printf (arg, stream, NULL, NULL, NULL); >>>> +} >> >> There's no reason to keep ui_printf around, just inline this into the 2 >> callers. >> >>>> +extern void printf_command (char *arg, int from_tty); >>>> +typedef void (printf_callback) (char **expp, struct bp_location *loc, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct agent_expr *aexpr); >>>> +extern void string_printf (char *arg, struct ui_file *stream, >>>> + ? ? ? ? ? ? ? ? ? ? ? printf_callback callback, struct bp_location *loc, >>>> + ? ? ? ? ? ? ? ? ? ? ? struct agent_expr *aexpr); >> >> These should be in some appropriate header, not tracepoint.c. >> >>>> + ? ? ?/* The agent expr include expr for arguments, format string, 1 byte >>>> + ? ? ? * for aop_printf, 1 byte for the number of arguments, 1 byte for >>>> + ? ? ? * size of format string, 1 byte for blank after format string >>>> + ? ? ? * and 1 byte for aop_end. ?*/ >> >> Wrong comment format, no leading "*"s. >> >> >> This new feature needs a documentation patch and probably also a patch >> to NEWS. >> >> From what I can tell from the patch, the idea here is to add a printf to >> a tracepoint's actions, with the printf evaluated on the remote side. ?I >> guess that is an ok idea, though I don't use this stuff enough to really >> have an opinion. >> >> I think it would be good for other maintainers to speak up now. ?If it >> is left to me, I will allow this if you fix up the problems and write >> the documentation. >> >> Tom >> > > Thanks for your help Tom. > > I make a new patch according to your comments. ?And I have send the > patch for doc in prev mail. > > Best, > Hui > > 2011-02-17 ?Hui Zhu ?<teawater@gmail.com> > > ? ? ? ?* Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h. > ? ? ? ?* ax-gdb.c (gen_printf_expr_callback): New function. > ? ? ? ?* ax-general.c (ax_memcpy): New function. > ? ? ? ?(aop_map): Add new entry for "printf". > ? ? ? ?(ax_print): Handle "printf". > ? ? ? ?(ax_reqs): Ditto. > ? ? ? ?* ax.h (agent_op): Add aop_printf. > ? ? ? ?(ax_memcpy): Forward declare. > ? ? ? ?* printcmd.c (printf_callback): New typedef. > ? ? ? ?(string_printf): New function from ui_printf. > ? ? ? ?(ui_printf): Call string_printf. > ? ? ? ?(printf_command): Remove static. > ? ? ? ?* printcmd.h: New file. > ? ? ? ?* tracepoint.c (printf_command, gen_printf_expr_callback, > ? ? ? ?printf_callback, string_printf): Forward declares. > ? ? ? ?(validate_actionline, encode_actions_1): handle printf_command. > Hi guys, Patch in attachment checked in. Thanks, Hui 2011-02-17 Hui Zhu <teawater@gmail.com> * Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h. * ax-gdb.c (gen_printf_expr_callback): New function. * ax-gdb.h (gen_printf_expr_callback): Forward declare. * ax-general.c (ax_memcpy): New function. (aop_map): Add new entry for "printf". (ax_print): Handle "printf". (ax_reqs): Ditto. * ax.h (ax_memcpy): Forward declare. * common/ax.def (invalid2): Removed. (printf): New entry. * printcmd.c (printcmd.h): New include. (string_printf): New function. (ui_printf): Removed. (printf_command): Remove static. Call string_printf. (eval_command): Call string_printf. * printcmd.h: New file. * tracepoint.c (printf_command, gen_printf_expr_callback, printf_callback, string_printf): Forward declares. (validate_actionline, encode_actions_1): handle printf_command.
Attachment:
tp_print.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |