From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15753 invoked by alias); 5 Jun 2009 21:20:40 -0000 Received: (qmail 15737 invoked by uid 22791); 5 Jun 2009 21:20:38 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_26,J_CHICKENPOX_75,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; Fri, 05 Jun 2009 21:20:33 +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 n55LKVM8010643 for ; Fri, 5 Jun 2009 17:20:31 -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 n55LKUuW026810; Fri, 5 Jun 2009 17:20:30 -0400 Received: from opsy.redhat.com (vpn-12-153.rdu.redhat.com [10.11.12.153]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n55LKTdr020707; Fri, 5 Jun 2009 17:20:30 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 1E257378615; Fri, 5 Jun 2009 15:20:29 -0600 (MDT) To: Sami Wagiaalla Cc: GDB Patches Subject: Re: [PATCH 2/2] namespace support References: <4A16BBC7.3080608@redhat.com> From: Tom Tromey Reply-To: tromey@redhat.com Date: Fri, 05 Jun 2009 21:20:00 -0000 In-Reply-To: <4A16BBC7.3080608@redhat.com> (Sami Wagiaalla's message of "Fri\, 22 May 2009 10\:50\:47 -0400") 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-06/txt/msg00133.txt.bz2 >>>>> "Sami" == Sami Wagiaalla writes: Sami> Import data structure ( struct using_direct) is limited to Sami> representing import statements where the destination is a parent Sami> of the source (true for anonymous namespaces). This patch removes Sami> this limitation, allowing each import to represent independent Sami> import source and destination. Thanks. Aside from some nits this looks pretty good. Is this just a "technical" patch? Or does it enable some new user-visible functionality? The former is fine, but if it is the latter, how about a test case? Sami> + * dwarf2read.c (read_import_statement): Properly set import location Sami> + and destenation. Typo, should be "destination". Sami> + * cp-support.h (cp_add_using, cp_add_using_directive): Now take char* Sami> + inner, char* outer arguments. Updated callers. I suppose these arguments ought to be renamed, now that you have renamed the struct fields... Sami> + char outer[outer_len+1]; We can't use VLAs in gdb. Instead, use alloca. Also, just FYI, the GNU coding style requires spaces around that "+". Sami> + strncpy(outer, name, outer_len); Space before open paren. Sami> - cp_add_using_directive (name, Sami> - previous_component == 0 Sami> - ? 0 : previous_component - 2, Sami> - next_component); Sami> + cp_add_using_directive (outer, name); Do we know for sure that NAME is \0-terminated here? It needs to be due to the change to cp_add_using_directive. Sami> + /* Figure out where the statement is being imported to */ Comment should be a sentence. Sami> + /* Sami> + Figure out what the scope of the imported die is and prepend it Sami> + to the name of the imported die Sami> + */ Ditto; also the formatting is wrong: no newline after the first * or before the last *. Sami> + if(strlen (imported_name_prefix) > 0){ Wrong brace placement. Tom