This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Sérgio Durigan Júnior <sergiodj at linux dot vnet dot ibm dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 04 Nov 2008 23:12:38 +0200
- Subject: Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
- References: <1225773079.24532.52.camel@miki>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Date: Tue, 04 Nov 2008 02:31:19 -0200
Thanks, but I cannot say I like the non-portable assumptions of this
design. For example:
> +void
> +clear_syscall_catchpoints_info (void)
> +{
> + struct breakpoint *b;
> +
> + ALL_BREAKPOINTS (b)
> + if (syscall_catchpoint_p (b))
> + b->syscall_number = UNKNOWN_SYSCALL;
> +} ^^^^^^^^^^^^^^^^^
Who said that a syscall is necessarily defined by some number?
More generally, let's say I'd like to implement support for this on
Windows -- how would I need to go about it?
> + /* We are in a entry breakpoint. */
> + entry_breakpoint,
> +
This comment is too obscure, IMO. Please make it more explicit; I
think you want to say that it's an entry to a syscall, right?
> + const char *syscall_name =
> + gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);
Does this mean the notion that a syscall is a number have crept into
gdbarch, which is supposed to be the most architecture-independent
part of GDB?
> + /* Checking if the user provided a syscall name or a number. */
> + if (isdigit (cur_name[0]))
Is the assumption that no name will ever begin with a digit
universally valid?
> + error (_("The number '%d' does not represent a valid syscall."),
^^^^^^^^^^^^^^^
"a known syscall", I'd say.
> + if (syscall_number == UNKNOWN_SYSCALL)
> + error (_("Invalid syscall name '%s'."), cur_name);
^^^^^^^
"Unknown" is more accurate.
> static void
> @@ -7441,6 +7792,7 @@ delete_command (char *arg, int from_tty)
> b->type != bp_shlib_event &&
> b->type != bp_thread_event &&
> b->type != bp_overlay_event &&
> + b->type != bp_entry_breakpoint &&
The previous lines used TABs and spaces for indentation, while your
line uses only spaces.
> + add_catch_command ("syscall", _("\
> +Catch calls to syscalls.\n\
"call to syscalls" is awkward. I suggest to drop the "calls to" part.
> +With an argument, catch only calls of that syscall."),
Ditto, suggest to drop the "calls of" part.
> + add_setshow_filename_cmd ("gdb_datadir", class_maintenance,
> + &gdb_datadir, _("Set GDB's datadir path."),
> + _("Show GDB's datadir path."),
> + _("\
> +When set, GDB uses the specified path to search for data files."),
> + NULL, NULL,
> + &maintenance_set_cmdlist,
> + &maintenance_show_cmdlist);
This should be described in the manual.