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] |
Hi Tom, Thanks for your review. On Wed, May 8, 2013 at 4:49 AM, Tom Tromey <tromey@redhat.com> wrote: >>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes: > > Hui> According to your comments. I did some update with these patches to > Hui> added special command -dprintf-insert to insert dprintf. Its format > Hui> is close to simple dprintf command: > Hui> -dprintf-insert LOCATION FORMAT ARG ARG ... > > I like this approach much more. > Thanks for doing it. > > Hui> +static char * > Hui> +mi_argv_to_format (int format_num, char **argv, int argc) > > This needs an introductory comment explaining the arguments and result. Add: /* Convert arguments in ARGV to the string in "format",argv,argv... and return it. */ > > Hui> + /* If all the string need convert to \ddd mode, so * 2. > Hui> + + 2 for two ". > Hui> + + 1 for \0. */ > Hui> + format_size = strlen (argv[format_num]) * 4 + 3; > > The comment says "* 2" but the code says "* 4". > Personally I'd just use an obstack rather than mess around with explicit > reallocs and the like. Obstack is better than what I did in before. Thanks for that. Updated patch for it. > > Hui> + sprintf (format + format_current_size, "\\%o", > Hui> + argv[format_num][i]); > > It seems that this could do the wrong thing for a char that > sign-extends. I changed this part to: sprintf (tmp, "\\%o", (unsigned char)argv[format_num][i]); > > Hui> + /* Apply other argv to FORMAT. */ > Hui> + for (i = format_num + 1; i < argc; i++) > > It seems to me that it would be better to just pass in argv+argc to > mi_argv_to_format, and omit the format_num argument entirely. Fixed. > > Hui> +static void > Hui> +mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc) > > Intro comment. /* Insert breakpoint. If dprintf is true, it will insert dprintf. If not, it will insert other type breakpoint. */ > > Hui> + extra_string = mi_argv_to_format (oind + 1, argv, argc); > Hui> + make_cleanup (xfree, extra_string); > > This makes a dangling cleanup. Fixed. > > Hui> + if (tracepoint) > Hui> + { > Hui> + /* Note that to request a fast tracepoint, the client uses the > Hui> + "hardware" flag, although there's nothing of hardware related to > Hui> + fast tracepoints -- one can implement slow tracepoints with > Hui> + hardware breakpoints, but fast tracepoints are always software. > Hui> + "fast" is a misnomer, actually, "jump" would be more appropriate. > Hui> + A simulator or an emulator could conceivably implement fast > Hui> + regular non-jump based tracepoints. */ > Hui> + type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint; > Hui> + ops = &tracepoint_breakpoint_ops; > Hui> + } > Hui> + else if (dprintf) > Hui> + { > > It seems that 'tracepoint' and 'dprintf' are exclusive, so this should > be checked above where "hardware" is checked. Fixed. I post the new patches that updated according to your comments. Thanks, Hui 2013-05-10 Hui Zhu <hui@codesourcery.com> * breakpoint.c (dprintf_breakpoint_ops): Remove its static. * breakpoint.h (dprintf_breakpoint_ops): Add extern. * mi/mi-cmd-break.c (ctype.h): New include. (gdb_obstack.h): New include. (mi_argv_to_format, mi_cmd_break_insert_1): New. (mi_cmd_break_insert): Call mi_cmd_break_insert_1. (mi_cmd_dprintf_insert): New. * mi/mi-cmds.c (mi_cmds): Add "dprintf-insert". * mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern. 2013-05-10 Hui Zhu <hui@codesourcery.com> * gdb.texinfo (GDB/MI Breakpoint Commands): Describe the "-dprintf-insert" command. 2013-05-10 Hui Zhu <hui@codesourcery.com> * gdb.mi/Makefile.in (PROGS): Add "mi-dprintf". * gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.
Attachment:
mi-dprintf.txt
Description: Text document
Attachment:
mi-dprintf-doc.txt
Description: Text document
Attachment:
mi-dprintf-test.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |