From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9221 invoked by alias); 14 Apr 2009 00:29:03 -0000 Received: (qmail 9210 invoked by uid 22791); 14 Apr 2009 00:29:02 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Apr 2009 00:28:56 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3E0SfL2008780; Mon, 13 Apr 2009 20:28:41 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n3E0Sexo030725; Mon, 13 Apr 2009 20:28:40 -0400 Received: from opsy.redhat.com (vpn-13-0.rdu.redhat.com [10.11.13.0]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n3E0SdQ2013648; Mon, 13 Apr 2009 20:28:39 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id B6F7E50830B; Mon, 13 Apr 2009 18:28:38 -0600 (MDT) To: Abderrahim KITOUNI Cc: gdb-patches@sourceware.org Subject: Re: Re : New language support : Vala References: <3d6b0edb0902070607x29177016m48a40bd198b88f7e@mail.gmail.com> <3d6b0edb0902090505m6bcb142crab53b0f860535ff4@mail.gmail.com> <3d6b0edb0902182352p7143c29clb382aa5602720463@mail.gmail.com> <3d6b0edb0903061044m2aa8671cn55940f43bc7db9a9@mail.gmail.com> From: Tom Tromey Reply-To: Tom Tromey Date: Tue, 14 Apr 2009 00:29:00 -0000 In-Reply-To: <3d6b0edb0903061044m2aa8671cn55940f43bc7db9a9@mail.gmail.com> (Abderrahim KITOUNI's message of "Fri\, 6 Mar 2009 19\:44\:22 +0100") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2009-04/txt/msg00244.txt.bz2 >>>>> "Abderrahim" == Abderrahim KITOUNI writes: Sorry about the long delay on this. As a general rule, please send a patch ping message after a week or two without a response. This is currently the best way to ensure that your patch does not fall through the cracks. Tom> I don't know whether your FSF paperwork has gone through. I don't Tom> have a way to look that up any more; maybe someone else could find Tom> out. Abderrahim> Yes, I received the letter. Ok. The important bit for us is knowing when the FSF says that you are all set up. Tom> + target_read_string (value_as_address (name), &type_name, 100, NULL); Tom> That '100' looks odd. It would probably be better to use the new Tom> string-reading code that Thiago put in; see valprint.c:read_string. Abderrahim> I didn't find a better solution (read_string returns a Abderrahim> gdb_byte* and I don't know how to convert it). The result will be a string in the target encoding. You can convert it to the host encoding, assuming that is what you want, using the functions declared in charset.h. The patch is looking pretty good. You'll need to update it to work with cvs; some incompatible changes have gone in. A few specific notes: + char *_name; Don't start symbols with an underscore. + name = call_function_by_hand (g_type_name, 1, >ype); You may want to find a way to do this without an inferior call. First, this won't work for core files. Second, if the user puts a breakpoint in g_type_name, something weird will happen. +const struct op_print vala_op_print_tab[] = Should be static. +static struct field * +vala_type_get_fields (struct type *type, int *nfields) +{ + int offset = 0; + + if (TYPE_CODE (type) != TYPE_CODE_STRUCT) + /* error */ + return NULL; This can return NULL without setting *nfields, but... + fields = vala_type_get_fields (real_type, &nfields); + privfields = vala_type_get_private_fields (real_type, &nprivfields); + if (nfields + nprivfields > 0) ... this code does not check that. vala_type_get_private_fields has the same bug. I didn't look to see if this condition is checked before the call. If so then you can just call internal_error or error rather than return NULL. + if (VALA_TYPE_HAS_PRIV (type)) + { + offset = 2; + } Remove the braces here. + else if (TYPE_CODE (real_type) == TYPE_CODE_INT && + streq (TYPE_NAME (real_type), "char")) Put "&&" at the start of the line, not the end. + fputs_filtered (TYPE_CODE (type) == TYPE_CODE_STRUCT ? + "struct " : "enum ", stream); Ditto the "?" + /* Don't print typedefs that just remove the '_'. */ + if (streq (SYMBOL_NATURAL_NAME (new_symbol), TYPE_TAG_NAME (type) + 1)) + return; It seems like this should actually check for the "_". + if (TYPE_CODE (type) == TYPE_CODE_PTR && "&&" position. + TYPE_CODE (type = + check_typedef (TYPE_TARGET_TYPE (type))) == TYPE_CODE_STRUCT) GNU style prohibits embedded assignments like this. The linespec patch looks decent. I am not sure we want to do it this way, though. I think a new language function would be preferable. One nit: + result = strdup ("g_"); Should be xstrdup. Also, the new style is to put a static function above its users, and then not have a forward declaration for it. This is simpler to maintain. Too bad the Python support isn't further along. I'd like to see a new language supported purely using Python; this one would have been a good choice :-) I know it is a pain, but this really needs test cases and documentation -- documentation for the users, and test cases so the gdb developers can test it occasionally. ("Occasionally" since I assume most of us won't have the vala compiler installed.) Tom