From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68152 invoked by alias); 16 Mar 2015 19:55:04 -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 68142 invoked by uid 89); 16 Mar 2015 19:55:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 16 Mar 2015 19:55:03 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 28E881163C8; Mon, 16 Mar 2015 15:55:01 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 9xggnZFbQwIn; Mon, 16 Mar 2015 15:55:01 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 047791163C4; Mon, 16 Mar 2015 15:55:00 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 87DD640EAD; Mon, 16 Mar 2015 15:55:00 -0400 (EDT) Date: Mon, 16 Mar 2015 19:55:00 -0000 From: Joel Brobecker To: Keven Boell Cc: Keven Boell , gdb-patches@sourceware.org Subject: Re: [PATCH] Introduce linked list for dynamic attributes Message-ID: <20150316195500.GA3435@adacore.com> References: <1425281876-20302-1-git-send-email-keven.boell@intel.com> <20150302194959.GD4449@adacore.com> <54F5BA7A.2090308@linux.intel.com> <20150303155457.GA3243@adacore.com> <54F6FA0D.4000103@linux.intel.com> <20150305181347.GB4604@adacore.com> <54FEDD80.1020102@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54FEDD80.1020102@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-03/txt/msg00470.txt.bz2 > From: Keven Boell > Date: Mon, 23 Feb 2015 15:45:06 +0100 > Subject: [PATCH] Introduce linked list for dynamic attributes > > This patch introduces a linked list for dynamic > attributes of a type. This is a pre-work for > the Fortran dynamic array support. The Fortran > dynamic array support will add more dynamic > attributes to a type. As only a few types > will have such dynamic attributes set, a linked > list is more efficient in terms of memory > consumption than adding multiple attributes > to main_type. > > Transformed the data_location dynamic attribute > to use the linked list. > > 2015-02-23 Keven Boell > > * gdbtypes.c (resolve_dynamic_type_internal): > Adapt data_location usage to linked list. > (resolve_dynamic_type_internal): Adapt > data_location usage to linked list. > (get_dyn_prop): New function. > (add_dyn_prop): New function. > (copy_dynamic_prop_list): New function. > (copy_type_recursive): Add copy of linked > list. > (copy_type): Add copy of linked list. > * gdbtypes.h (enum dynamic_prop_node_kind): > Kind of the dynamic attribute in linked list. > (struct dynamic_prop_list): Dynamic list > node. > * dwarf2read.c (set_die_type): Add data_location > data to linked list using helper functions. OK to push, but before you do, make sure you fix up the date in the ChangeLog entry both in the revision history (the one quoted above) and the one in the ChangeLog file. Also, would you mind reformatting the revision log to something that uses a slightly longer line length? Something like 70 characters will allow us to use fewer lines, and make the revision log a little shorter (as well as more readable, IMO, but that may be just me). And while looking at the contents of the ChangeLog one more time, I think I missed on some suggestions. So below is what I suggest. Please take a look to see what the changes are and whether you agree with them. Basically, the ChangeLog says what you did in concise terms, now what the different new elements are for. | This patch introduces a linked list for dynamic attributes of a type. | This is a pre-work for the Fortran dynamic array support. The Fortran | dynamic array support will add more dynamic attributes to a type. | As only a few types will have such dynamic attributes set, a linked | list is more efficient in terms of memory consumption than adding | multiple attributes to main_type. | | 2015-03-16 Keven Boell | | * gdbtypes.c (resolve_dynamic_type_internal): Adapt | data_location usage to linked list. | (resolve_dynamic_type_internal): Adapt data_location to | linked list. | (get_dyn_prop, add_dyn_prop, copy_dynamic_prop_list): New function. | (copy_type_recursive, copy_type): Add copy of linked list. | * gdbtypes.h (enum dynamic_prop_node_kind): New enum. | (struct dynamic_prop_list): New struct. | * dwarf2read.c (set_die_type): Set data_location data. Please make sure that this ChangeLog entry matches what you put in gdb/ChangeLog. Other than that, well done, and thank you for the patch. We can now move to the next phase of your patch series! -- Joel