This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Introduce linked list for dynamic attributes


> From: Keven Boell <keven.boell@intel.com>
> 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  <keven.boell@intel.com>
> 
> 	* 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  <keven.boell@intel.com>
| 
| 	* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]