From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5281 invoked by alias); 7 Nov 2008 03:46:59 -0000 Received: (qmail 5190 invoked by uid 22791); 7 Nov 2008 03:46:58 -0000 X-Spam-Check-By: sourceware.org Received: from igw2.br.ibm.com (HELO igw2.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 07 Nov 2008 03:46:12 +0000 Received: from d24relay01.br.ibm.com (unknown [9.8.31.16]) by igw2.br.ibm.com (Postfix) with ESMTP id 3A3F117F59D for ; Fri, 7 Nov 2008 01:44:37 -0200 (BRDT) Received: from d24av02.br.ibm.com (d24av02.br.ibm.com [9.18.232.47]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mA73jmIi4206786 for ; Fri, 7 Nov 2008 00:45:48 -0300 Received: from d24av02.br.ibm.com (loopback [127.0.0.1]) by d24av02.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mA73k9YW027075 for ; Fri, 7 Nov 2008 01:46:09 -0200 Received: from [9.8.9.146] ([9.8.9.146]) by d24av02.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id mA73k9au027069; Fri, 7 Nov 2008 01:46:09 -0200 Subject: Re: [PATCH 3/4] 'catch syscall' feature -- XML support part From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= To: tromey@redhat.com Cc: gdb-patches@sourceware.org In-Reply-To: References: <1225773086.24532.55.camel@miki> Content-Type: text/plain; charset=iso-8859-1 Date: Fri, 07 Nov 2008 03:46:00 -0000 Message-Id: <1226029557.32321.104.camel@miki> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2008-11/txt/msg00117.txt.bz2 Hey again :-) On Tue, 2008-11-04 at 11:40 -0700, Tom Tromey wrote: > >>>>> "Sérgio" == Sérgio Durigan Júnior writes: > Sérgio> +struct syscalls_info > Sérgio> +{ > Sérgio> + /* The number of syscalls in this system. */ > Sérgio> + > Sérgio> + int number_of_syscalls; > Sérgio> + > Sérgio> + /* The syscalls. */ > Sérgio> + > Sérgio> + VEC(syscall_desc_p) *syscalls; > Sérgio> +}; > > I think number_of_syscalls is redundant here. The VEC knows how many > elements it contains, so why not just use that? Hmm, OK. I'm getting used with this VEC data structure, so I still don't know what it does/doesn't provide. Thanks for the hint. > > Sérgio> +static void > Sérgio> +do_cleanup_fclose (void *file) > Sérgio> +{ > Sérgio> + fclose (file); > Sérgio> +} > > There's a make_cleanup_fclose now. Didn't know too. Thanks. > Sérgio> + > [...] > Sérgio> + for (i = 0; i < len; i++) > Sérgio> + { > Sérgio> + if (strcmp (attrs[i].name, "name") == 0) > Sérgio> + name = attrs[i].value; > Sérgio> + else if (strcmp (attrs[i].name, "number") == 0) > Sérgio> + number = * (ULONGEST *) attrs[i].value; > Sérgio> + else > Sérgio> + internal_error (__FILE__, __LINE__, > Sérgio> + _("Unknown attribute name '%s'."), attrs[i].name); > Sérgio> + } > Sérgio> + > Sérgio> + syscall_create_syscall_desc (data->sysinfo, name, number); > > This will make an entry with an empty name given the above XML > snippet. But that doesn't seem helpful... if this triggers, won't it > print as an unnamed syscall? It seems to me that it would be > preferable to skip nameless ones and just report them numerically, if > they occur. I'm sorry, but I think I didn't understand completely what you suggested. You mean that I should set the syscall name to NULL when there's no name for it in the XML file? > Sérgio> +static const struct gdb_xml_attribute syscall_attr[] = { > Sérgio> + { "number", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL }, > > The indentation is wrong here and in other definitions like this. > It should be just 2 spaces. Ha, I was sure that I'd receive some complains about the indentation :-P. > Sérgio> +static struct syscalls_info * > Sérgio> +syscall_parse_xml (const char *document, xml_fetch_another fetcher, > Sérgio> + void *fetcher_baton) > Sérgio> +{ > [...] > Sérgio> + back_to = make_cleanup (null_cleanup, NULL); > > I don't think you need to make a null cleanup here... Right. > Sérgio> + if (gdb_xml_parse (parser, document) == 0) > Sérgio> + { > Sérgio> + /* Parsed successfully. */ > Sérgio> + discard_cleanups (result_cleanup); > Sérgio> + do_cleanups (back_to); > Sérgio> + return data.sysinfo; > Sérgio> + } > Sérgio> + else > Sérgio> + { > Sérgio> + warning (_("Could not load XML syscalls info; ignoring")); > Sérgio> + do_cleanups (back_to); > Sérgio> + return NULL; > > ... you can either just do_ or discard_ result_cleanup instead. I'm sorry, instead of what? do_cleanups? :-) Thanks again! Regards, -- Sérgio Durigan Júnior Linux on Power Toolchain - Software Engineer Linux Technology Center - LTC IBM Brazil