From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5928 invoked by alias); 8 Oct 2014 19:07:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 5911 invoked by uid 89); 8 Oct 2014 19:07:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 08 Oct 2014 19:07:53 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s98J7neR026655 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 8 Oct 2014 15:07:50 -0400 Received: from localhost (dhcp-10-15-16-169.yyz.redhat.com [10.15.16.169]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s98J7mqN029615 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 8 Oct 2014 15:07:49 -0400 From: Sergio Durigan Junior To: Gabriel Krisman Bertazi Cc: gdb-patches@sourceware.org Subject: Re: [RFC PATCH 2/3] Add support to catch groups of syscalls. References: <1412736678-2760-1-git-send-email-gabriel@krisman.be> <1412736678-2760-3-git-send-email-gabriel@krisman.be> X-URL: http://blog.sergiodj.net Date: Wed, 08 Oct 2014 19:07:00 -0000 In-Reply-To: <1412736678-2760-3-git-send-email-gabriel@krisman.be> (Gabriel Krisman Bertazi's message of "Tue, 7 Oct 2014 23:51:17 -0300") Message-ID: <87h9zebcsb.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00168.txt.bz2 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 http://sergiodj.net/