This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] add -s option to make -break-insert support dprintf
- From: Tom Tromey <tromey at redhat dot com>
- To: Hui Zhu <teawater at gmail dot com>
- Cc: Pedro Alves <palves at redhat dot com>, Eli Zaretskii <eliz at gnu dot org>, Hui Zhu <hui_zhu at mentor dot com>, gdb-patches ml <gdb-patches at sourceware dot org>, Marc Khouzam <marc dot khouzam at ericsson dot com>
- Date: Tue, 07 May 2013 14:49:49 -0600
- Subject: Re: [PATCH] add -s option to make -break-insert support dprintf
- References: <515451EA dot 1000200 at mentor dot com> <83y5d7wpvq dot fsf at gnu dot org> <CANFwon23qn_SVjcUWUZ2Z2Y5Euqg8efiwMvXkxTRtA9-2Ttk3Q at mail dot gmail dot com> <516454DA dot 9040109 at redhat dot com> <CANFwon1aDoyCYrsNeUpkmh6ARFJmT8B4JdFqYcc6GLdo=cgqig at mail dot gmail dot com> <87ppxzhfqy dot fsf at fleche dot redhat dot com> <CANFwon2_yT4SOpK7=Rq=91nFcvk2Rn3_wAkCMmsfgP-6iynGig at mail dot gmail dot com> <516C2549 dot 3060808 at redhat dot com> <CANFwon0WYLZEKWkfHzRgCB8MOPPoCsDvSaCk0SSYXPgonuE5zw at mail dot gmail dot com> <87vc7ithtj dot fsf at fleche dot redhat dot com> <CANFwon0jwdrDULV+bPRX_5AWE2tgq=rSrA6eyZEujZqPfN+Huw at mail dot gmail dot com> <87wqrrll9m dot fsf at fleche dot redhat dot com> <CANFwon3yyKxMVRO3wJ3=VTDLpege_NWByMA9-jk_g9_a5mXaoQ at mail dot gmail dot com>
>>>>> "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.
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.
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.
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.
Hui> +static void
Hui> +mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
Intro comment.
Hui> + extra_string = mi_argv_to_format (oind + 1, argv, argc);
Hui> + make_cleanup (xfree, extra_string);
This makes a dangling cleanup.
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.
Tom