From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110293 invoked by alias); 22 Jun 2016 19:10:32 -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 110266 invoked by uid 89); 22 Jun 2016 19:10:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:2238, route, HTo:D*mozilla.com X-HELO: gproxy2-pub.mail.unifiedlayer.com Received: from gproxy2-pub.mail.unifiedlayer.com (HELO gproxy2-pub.mail.unifiedlayer.com) (69.89.18.3) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with SMTP; Wed, 22 Jun 2016 19:10:19 +0000 Received: (qmail 18680 invoked by uid 0); 22 Jun 2016 19:10:18 -0000 Received: from unknown (HELO CMOut01) (10.0.90.82) by gproxy2.mail.unifiedlayer.com with SMTP; 22 Jun 2016 19:10:18 -0000 Received: from box522.bluehost.com ([74.220.219.122]) by CMOut01 with id 9vAD1t0182f2jeq01vAGEu; Wed, 22 Jun 2016 13:10:16 -0600 X-Authority-Analysis: v=2.1 cv=OPe0g0qB c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=PnD2wP_eR3oA:10 a=7XZj0uCbPdcA:10 a=pD_ry4oyNxEA:10 a=BzObnAOqAAAA:8 a=L9hoXDQPSO8re72reM8A:9 a=PuSIgb6VMkSJ0_1bSHE4:22 Received: from [75.171.172.174] (port=45106 helo=pokyo) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.86_2) (envelope-from ) id 1bFnXS-0003E9-KH; Wed, 22 Jun 2016 13:10:14 -0600 From: Tom Tromey To: Manish Goregaokar Cc: gdb-patches@sourceware.org, Tom Tromey Subject: Re: [PATCH 1/2][PR gdb/20239] Make evaluation and type-printing of all NonZero-optimized enums work References: Date: Wed, 22 Jun 2016 19:10:00 -0000 In-Reply-To: (Manish Goregaokar's message of "Tue, 21 Jun 2016 15:10:28 +0530") Message-ID: <87vb11jazw.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.95 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Identified-User: {36111:box522.bluehost.com:elynrobi:tromey.com} {sentby:smtp auth 75.171.172.174 authed with tom+tromey.com} X-Exim-ID: 1bFnXS-0003E9-KH X-Source-Sender: (pokyo) [75.171.172.174]:45106 X-Source-Auth: tom+tromey.com X-Email-Count: 0 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-SW-Source: 2016-06/txt/msg00372.txt.bz2 >>>>> "Manish" == Manish Goregaokar writes: Manish> Regarding the xstrdup, this is done because strsep edits the string Manish> itself. Indeed it does. Somehow I didn't realize this, sorry about that. Manish> Methods tests fail, but they are Manish> a preexisting failure due to Rust's debuginfo changing. Yes, the situation with 1.9 is not that good :( This patch looks good. There are formatting nits but really nothing serious. The normal procedure is that once you've written one good patch you can get write-after-approval access to gdb. If you want to go this route, let me know. Otherwise (and also let me know) I can push the final patches for you. Manish> + char *type_name; I think this is assigned to but not used, so you might as well remove it. Manish> + /* Optimized enums have only one field */ gdb style is period with two spaces at the end of a comment. Manish> + while ((token = strsep (&tail, "$")) != NULL) Manish> + { The "{" should be indented 2 more spaces, and then the body as well. Manish> + if (sscanf (token, "%lu", &fieldno) != 1) Manish> + { Here too. Manish> + /* We have reached the enum name, which cannot start with a digit */ The period thing again. Also this line might be too long. Manish> + }; Extra ";". Manish> + value = unpack_long (member_type, Manish> + valaddr + embedded_offset); Make sure the continuation line is indented to line up under "member_type". (Maybe it is but I can't tell ...) Manish> int i, len = 0; Manish> + int skip_to = 1; /* Skip the discriminant field */ Indentation? Also gdb doesn't tend to use trailing comments like that; you can put it before the declaration. Manish> + char *zero_field = strrchr (TYPE_FIELD_NAME (type, 0), '$'); const char * Manish> + if (zero_field != NULL && strlen (zero_field) > 1) { "{" on the next line and indented. Manish> + fprintfi_filtered (level+2, stream, "%s,\n", zero_field+1); Spaces around "+"s. Manish> + /* there is no explicit discriminant field, skip nothing */ Upper case "There", period. Manish> - int first = 1; Manish> + int first = 1; Some spurious change maybe? Tom