Re: [RFC PATCH 2/3] Add support to catch groups of syscalls.

Sergio Durigan Junior
2014-10-08T19:07:55Z

On Tuesday, October 07 2014, Gabriel Krisman Bertazi wrote:

> This implements the catchpoint side. While parsing 'catch syscall'
> arguments, we verify if the argument is a syscall group and expand it to
> a list of syscalls that are part of that group.

Thanks for the patch. Comments below.

> gdb/
>
> * breakpoint.c (catch_syscall_split_args): Verify if argument
> is a syscall group and expand it to a list of syscalls when
> creating catchpoints.
> (catch_syscall_completer): Include syscall groups to the list of
> word completion.
> ---
> gdb/breakpoint.c | 73 +++++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 57 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e2170b4..5243916 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -12117,22 +12117,50 @@ catch_syscall_split_args (char *arg)
> /* Check if the user provided a syscall name or a number. */
> syscall_number = (int) strtol (cur_name, &endptr, 0);
> if (*endptr == '\0')
> - get_syscall_by_number (syscall_number, &s);
> + {
> + get_syscall_by_number (syscall_number, &s);
> +
> + /* Ok, it's valid. */
> + VEC_safe_push (int, result, s.number);
> + }
> else
> {
> - /* We have a name. Let's check if it's valid and convert it
> - to a number. */
> - get_syscall_by_name (cur_name, &s);
> -
> - if (s.number == UNKNOWN_SYSCALL)
> - /* Here we have to issue an error instead of a warning,
> - because GDB cannot do anything useful if there's no
> - syscall number to be caught. */
> + const char **syscall_list;
> +
> + /* If we have a syscall group, expand it to a list of
> + syscalls. */
> + syscall_list = get_syscall_names_by_group (cur_name);
> +
> + if (syscall_list == NULL)
> error (_("Unknown syscall name '%s'."), cur_name);

This part...

> - }
>
> - /* Ok, it's valid. */
> - VEC_safe_push (int, result, s.number);
> + if (syscall_list[0] == NULL)
> + {
> + /* If syscall_list is empty, cur_name is a syscall name
> + instead of a group. We push it into the group
> + list. */
> + syscall_list[0] = cur_name;
> + syscall_list[1] = NULL;
> + }

... and this part are the reasons I made the comment on the first patch
about leaving to much to the caller. For example, the caller has to
know that syscall_list == NULL and *syscall_list == NULL are different
things which mean entirely different things. However, *syscall_list ==
NULL should not happen in this interface, as I see it.

As I mentioned, you're doing this here because you have no way to know
whether the user is asking to catch a syscall name or a group of
syscalls. This would be solved with my early proposal, of implementing
the "-g" modifier on the catch syscall command. It would also make the
code simpler to follow here.

> +
> + for (i = 0; syscall_list[i]; i++)
> + {
> + get_syscall_by_name (syscall_list[i], &s);
> +
> + if (s.number == UNKNOWN_SYSCALL)
> + {
> + /* Here we have to issue an error instead of a warning,
> + because GDB cannot do anything useful if there's no
> + syscall number to be caught. */
> + error (_("Unknown syscall name '%s'."), syscall_list[i]);
> + }
> +
> + /* Ok, it's valid. */
> + VEC_safe_push (int, result, s.number);
> + }
> +
> + xfree (syscall_list);
> + }
> }
>
> discard_cleanups (cleanup);
> @@ -15615,11 +15643,24 @@ static VEC (char_ptr) *
> catch_syscall_completer (struct cmd_list_element *cmd,
> const char *text, const char *word)
> {
> - const char **list = get_syscall_names ();
> - VEC (char_ptr) *retlist
> - = (list == NULL) ? NULL : complete_on_enum (list, word, word);
> + VEC (char_ptr) *retlist;
> + const char **syscall_list = get_syscall_names ();
> + const char **group_list = get_syscall_group_names ();
> +
> + VEC (char_ptr) *sys_retlist
> + = (syscall_list == NULL) ? NULL : complete_on_enum (syscall_list,
> + word, word);
> +
> + VEC (char_ptr) *group_retlist
> + = (group_list == NULL) ? NULL : complete_on_enum (group_list, word, word);

No newlines between variables being declared.

> +
> + retlist = VEC_merge (char_ptr, sys_retlist, group_retlist);
> +
> + xfree (syscall_list);
> + xfree (group_list);
> + VEC_free (char_ptr, sys_retlist);
> + VEC_free (char_ptr, group_retlist);
>
> - xfree (list);
> return retlist;
> }
>
> --
> 1.9.3

--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible