* Don't assume order of xml attributes
@ 2011-02-02 16:19 Pedro Alves
2011-02-02 16:34 ` Daniel Jacobowitz
0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2011-02-02 16:19 UTC (permalink / raw)
To: gdb-patches
Something I noticed by inspection while adding support for
a new xml file fetched with qXfer:$object.
XML attributes are not ordered. We should not
rely on attribute order for anything. This patch
adds a helper function to get at a given attribute by name,
and updates all places to use it instead of refering to
attributes by index.
Applied.
--
Pedro Alves
2011-02-02 Pedro Alves <pedro@codesourcery.com>
gdb/
* xml-support.c (xml_find_attribute): New.
(xinclude_start_include): Use it.
* xml-support.h (xml_find_attribute): Declare.
* memory-map.c (memory_map_start_memory)
(memory_map_start_property): Use xml_find_attribute.
* osdata.c (osdata_start_osdata, osdata_start_column): Use
xml_find_attribute.
* remote.c (start_thread): Use xml_find_attribute.
* solib-target.c (library_list_start_segment)
(library_list_start_section, library_list_start_library)
(library_list_start_list): Use xml_find_attribute.
* xml-tdesc.c (tdesc_start_target, tdesc_start_feature)
(tdesc_start_union, tdesc_start_struct, tdesc_start_flags)
(tdesc_start_field): Use xml_find_attribute.
---
gdb/memory-map.c | 8 ++++----
gdb/osdata.c | 4 ++--
gdb/remote.c | 9 +++++----
gdb/solib-target.c | 8 ++++----
gdb/xml-support.c | 18 +++++++++++++++++-
gdb/xml-support.h | 6 ++++++
gdb/xml-tdesc.c | 41 +++++++++++++++++++++--------------------
7 files changed, 59 insertions(+), 35 deletions(-)
Index: src/gdb/xml-support.c
===================================================================
--- src.orig/gdb/xml-support.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/xml-support.c 2011-02-02 15:55:04.377898996 +0000
@@ -138,6 +138,22 @@ gdb_xml_error (struct gdb_xml_parser *pa
throw_verror (XML_PARSE_ERROR, format, ap);
}
+/* Find the attribute named NAME in the set of parsed attributes
+ ATTRIBUTES. Returns NULL if not found. */
+
+struct gdb_xml_value *
+xml_find_attribute (VEC(gdb_xml_value_s) *attributes, const char *name)
+{
+ struct gdb_xml_value *value;
+ int ix;
+
+ for (ix = 0; VEC_iterate (gdb_xml_value_s, attributes, ix, value); ix++)
+ if (strcmp (value->name, name) == 0)
+ return value;
+
+ return NULL;
+}
+
/* Clean up a vector of parsed attribute values. */
static void
@@ -758,7 +774,7 @@ xinclude_start_include (struct gdb_xml_p
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct xinclude_parsing_data *data = user_data;
- char *href = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *href = xml_find_attribute (attributes, "href")->value;
struct cleanup *back_to;
char *text, *output;
Index: src/gdb/xml-support.h
===================================================================
--- src.orig/gdb/xml-support.h 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/xml-support.h 2011-02-02 15:55:23.007898997 +0000
@@ -220,6 +220,12 @@ void gdb_xml_debug (struct gdb_xml_parse
void gdb_xml_error (struct gdb_xml_parser *parser, const char *format, ...)
ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
+/* Find the attribute named NAME in the set of parsed attributes
+ ATTRIBUTES. Returns NULL if not found. */
+
+struct gdb_xml_value *xml_find_attribute (VEC(gdb_xml_value_s) *attributes,
+ const char *name);
+
/* Parse an integer attribute into a ULONGEST. */
extern gdb_xml_attribute_handler gdb_xml_parse_attr_ulongest;
Index: src/gdb/memory-map.c
===================================================================
--- src.orig/gdb/memory-map.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/memory-map.c 2011-02-02 15:54:05.937898995 +0000
@@ -64,9 +64,9 @@ memory_map_start_memory (struct gdb_xml_
struct mem_region *r = VEC_safe_push (mem_region_s, *data->memory_map, NULL);
ULONGEST *start_p, *length_p, *type_p;
- start_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;
- length_p = VEC_index (gdb_xml_value_s, attributes, 1)->value;
- type_p = VEC_index (gdb_xml_value_s, attributes, 2)->value;
+ start_p = xml_find_attribute (attributes, "start")->value;
+ length_p = xml_find_attribute (attributes, "length")->value;
+ type_p = xml_find_attribute (attributes, "type")->value;
mem_region_init (r);
r->lo = *start_p;
@@ -101,7 +101,7 @@ memory_map_start_property (struct gdb_xm
struct memory_map_parsing_data *data = user_data;
char *name;
- name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ name = xml_find_attribute (attributes, "name")->value;
snprintf (data->property_name, sizeof (data->property_name), "%s", name);
}
Index: src/gdb/osdata.c
===================================================================
--- src.orig/gdb/osdata.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/osdata.c 2011-02-02 15:54:05.937898995 +0000
@@ -68,7 +68,7 @@ osdata_start_osdata (struct gdb_xml_pars
if (data->osdata)
gdb_xml_error (parser, _("Seen more than on osdata element"));
- type = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ type = xml_find_attribute (attributes, "type")->value;
osdata = XZALLOC (struct osdata);
osdata->type = xstrdup (type);
data->osdata = osdata;
@@ -95,7 +95,7 @@ osdata_start_column (struct gdb_xml_pars
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct osdata_parsing_data *data = user_data;
- const char *name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ const char *name = xml_find_attribute (attributes, "name")->value;
data->property_name = xstrdup (name);
}
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/remote.c 2011-02-02 15:54:05.937898995 +0000
@@ -2505,13 +2505,14 @@ start_thread (struct gdb_xml_parser *par
struct thread_item item;
char *id;
+ struct gdb_xml_value *attr;
- id = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ id = xml_find_attribute (attributes, "id")->value;
item.ptid = read_ptid (id, NULL);
- if (VEC_length (gdb_xml_value_s, attributes) > 1)
- item.core = *(ULONGEST *) VEC_index (gdb_xml_value_s,
- attributes, 1)->value;
+ attr = xml_find_attribute (attributes, "core");
+ if (attr != NULL)
+ item.core = *(ULONGEST *) attr->value;
else
item.core = -1;
Index: src/gdb/solib-target.c
===================================================================
--- src.orig/gdb/solib-target.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/solib-target.c 2011-02-02 15:54:05.937898995 +0000
@@ -86,7 +86,7 @@ library_list_start_segment (struct gdb_x
{
VEC(lm_info_p) **list = user_data;
struct lm_info *last = VEC_last (lm_info_p, *list);
- ULONGEST *address_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ ULONGEST *address_p = xml_find_attribute (attributes, "address")->value;
CORE_ADDR address = (CORE_ADDR) *address_p;
if (last->section_bases != NULL)
@@ -103,7 +103,7 @@ library_list_start_section (struct gdb_x
{
VEC(lm_info_p) **list = user_data;
struct lm_info *last = VEC_last (lm_info_p, *list);
- ULONGEST *address_p = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ ULONGEST *address_p = xml_find_attribute (attributes, "address")->value;
CORE_ADDR address = (CORE_ADDR) *address_p;
if (last->segment_bases != NULL)
@@ -122,7 +122,7 @@ library_list_start_library (struct gdb_x
{
VEC(lm_info_p) **list = user_data;
struct lm_info *item = XZALLOC (struct lm_info);
- const char *name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ const char *name = xml_find_attribute (attributes, "name")->value;
item->name = xstrdup (name);
VEC_safe_push (lm_info_p, *list, item);
@@ -150,7 +150,7 @@ library_list_start_list (struct gdb_xml_
const struct gdb_xml_element *element,
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
- char *version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *version = xml_find_attribute (attributes, "version")->value;
if (strcmp (version, "1.0") != 0)
gdb_xml_error (parser,
Index: src/gdb/xml-tdesc.c
===================================================================
--- src.orig/gdb/xml-tdesc.c 2011-02-02 15:50:11.877898999 +0000
+++ src/gdb/xml-tdesc.c 2011-02-02 15:54:05.947898996 +0000
@@ -152,7 +152,7 @@ tdesc_start_target (struct gdb_xml_parse
const struct gdb_xml_element *element,
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
- char *version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *version = xml_find_attribute (attributes, "version")->value;
if (strcmp (version, "1.0") != 0)
gdb_xml_error (parser,
@@ -168,7 +168,7 @@ tdesc_start_feature (struct gdb_xml_pars
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct tdesc_parsing_data *data = user_data;
- char *name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *name = xml_find_attribute (attributes, "name")->value;
data->current_feature = tdesc_create_feature (data->tdesc, name);
}
@@ -233,7 +233,7 @@ tdesc_start_union (struct gdb_xml_parser
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct tdesc_parsing_data *data = user_data;
- char *id = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *id = xml_find_attribute (attributes, "id")->value;
data->current_type = tdesc_create_union (data->current_feature, id);
data->current_type_size = 0;
@@ -249,18 +249,19 @@ tdesc_start_struct (struct gdb_xml_parse
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct tdesc_parsing_data *data = user_data;
- char *id = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *id = xml_find_attribute (attributes, "id")->value;
struct tdesc_type *type;
+ struct gdb_xml_value *attr;
type = tdesc_create_struct (data->current_feature, id);
data->current_type = type;
data->current_type_size = 0;
data->current_type_is_flags = 0;
- if (VEC_length (gdb_xml_value_s, attributes) > 1)
+ attr = xml_find_attribute (attributes, "size");
+ if (attr != NULL)
{
- int size = (int) * (ULONGEST *)
- VEC_index (gdb_xml_value_s, attributes, 1)->value;
+ int size = (int) * (ULONGEST *) attr->value;
tdesc_set_struct_size (type, size);
data->current_type_size = size;
@@ -273,9 +274,9 @@ tdesc_start_flags (struct gdb_xml_parser
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct tdesc_parsing_data *data = user_data;
- char *id = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+ char *id = xml_find_attribute (attributes, "id")->value;
int length = (int) * (ULONGEST *)
- VEC_index (gdb_xml_value_s, attributes, 1)->value;
+ xml_find_attribute (attributes, "size")->value;
struct tdesc_type *type;
type = tdesc_create_flags (data->current_feature, id, length);
@@ -294,28 +295,28 @@ tdesc_start_field (struct gdb_xml_parser
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
struct tdesc_parsing_data *data = user_data;
- int ix = 0, length;
- struct gdb_xml_value *attrs = VEC_address (gdb_xml_value_s, attributes);
+ struct gdb_xml_value *attr;
struct tdesc_type *field_type;
char *field_name, *field_type_id;
int start, end;
- length = VEC_length (gdb_xml_value_s, attributes);
+ field_name = xml_find_attribute (attributes, "name")->value;
- field_name = attrs[ix++].value;
-
- if (ix < length && strcmp (attrs[ix].name, "type") == 0)
- field_type_id = attrs[ix++].value;
+ attr = xml_find_attribute (attributes, "type");
+ if (attr != NULL)
+ field_type_id = attr->value;
else
field_type_id = NULL;
- if (ix < length && strcmp (attrs[ix].name, "start") == 0)
- start = * (ULONGEST *) attrs[ix++].value;
+ attr = xml_find_attribute (attributes, "start");
+ if (attr != NULL)
+ start = * (ULONGEST *) attr->value;
else
start = -1;
- if (ix < length && strcmp (attrs[ix].name, "end") == 0)
- end = * (ULONGEST *) attrs[ix++].value;
+ attr = xml_find_attribute (attributes, "end");
+ if (attr != NULL)
+ end = * (ULONGEST *) attr->value;
else
end = -1;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Don't assume order of xml attributes
2011-02-02 16:19 Don't assume order of xml attributes Pedro Alves
@ 2011-02-02 16:34 ` Daniel Jacobowitz
2011-02-02 17:00 ` Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Jacobowitz @ 2011-02-02 16:34 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, Feb 02, 2011 at 04:19:36PM +0000, Pedro Alves wrote:
> Something I noticed by inspection while adding support for
> a new xml file fetched with qXfer:$object.
>
> XML attributes are not ordered. We should not
> rely on attribute order for anything. This patch
> adds a helper function to get at a given attribute by name,
> and updates all places to use it instead of refering to
> attributes by index.
This was not required. The attributes don't have to be ordered in the
XML file, but they're ordered in the VEC because we create it that way
in gdb_start_element. You can tell your patch still relies on this
code because the calls to the lookup function mostly don't check for
NULL.
That said, I don't think a linear search for attribute names is going
to be too slow :-) So this is a nice readability improvement anyway.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Don't assume order of xml attributes
2011-02-02 16:34 ` Daniel Jacobowitz
@ 2011-02-02 17:00 ` Pedro Alves
0 siblings, 0 replies; 3+ messages in thread
From: Pedro Alves @ 2011-02-02 17:00 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
On Wednesday 02 February 2011 16:33:52, Daniel Jacobowitz wrote:
> This was not required. The attributes don't have to be ordered in the
> XML file, but they're ordered in the VEC because we create it that way
> in gdb_start_element. You can tell your patch still relies on this
> code because the calls to the lookup function mostly don't check for
> NULL.
Ah! Thanks for pointing it out. I completely missed it.
>
> That said, I don't think a linear search for attribute names is going
> to be too slow :-)
One would hope. :-)
> So this is a nice readability improvement anyway.
Yeah, I agree. I'll keep it in then.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-02 17:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-02 16:19 Don't assume order of xml attributes Pedro Alves
2011-02-02 16:34 ` Daniel Jacobowitz
2011-02-02 17:00 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox