[RFC] Make version attribute of <target> really optional

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

Hello,

I'm trying to fix the type of the "lr" register in the XML features
files for rs6000, but I have hit a strange problem: The XML parser
is complaining that the version number in the <target> element is
missing. On the other hand, the documentation says that it is
optional (but recommended).

Looking back at the discussion, it seems that the attribute was
meant to be optional for now. But on the other hand, the implementation
was such that the attribute is NOT optional.

I thought it'd quickly give it a go at making the attribute optional,
and the following seems to be working, but now I'm having other issues
when trying to read the XML file from GDB:

    (gdb) set tdesc filename rs6000/powerpc-32.xml 
    warning: while parsing target description: unbound prefix
    warning: Could not load XML target description; ignoring

This error seems independent of the change I made, as I can reproduce
the same error without my change. It looks like the problem is happening
during the processing of the xi:include tag, but I can't figure out
better what is going on for lack of familiarity with the XML parser.

Just for the record, I'm using expat-2.0.1.

"set debug xml" reveals:

    (gdb) set debug xml 1
    (gdb) set tdesc filename rs6000/powerpc-32.xml
    target description (line 1): Starting:
    <?xml version="1.0"?>
    [snip]
    
    <!DOCTYPE target SYSTEM "gdb-target.dtd">
    <target>
      <architecture>powerpc:common</architecture>
      <xi:include href="power-core.xml"/>
      <xi:include href="power-fpu.xml"/>
      <xi:include href="power-altivec.xml"/>
    </target>
    
    target description (line 12): Entering element <target>
    target description (line 13): Entering element <architecture>
    target description (line 13): Leaving element <architecture>
    warning: while parsing target description: unbound prefix
    warning: Could not load XML target description; ignoring

We raised the failure because we weren't able to find element
"target" in the list of elements passed to us by the parser:

  /* Find this element in the list of the current scope's allowed
     children.  Record that we've seen it.  */

  seen = 1;
  for (element = scope->elements; element && element->name;
       element++, seen <<= 1)
    if (strcmp (element->name, name) == 0)
      break;

It feels like I'm missing something, here... Does it work for someone
else? Daniel, or Ulrich, maybe? You regenerated the files not long ago,
so perhaps it works for you...

The following is the untested patch that I wrote in an attempt to make
the version optional for now:

2008-02-28  Joel Brobecker  <brobecker@adacore.com>

        * xml-tdesc.c (tdesc_start_target): Handle the "version" attribute
        as optional.
        (target_attributes): Make the "version" attribute optional.

Note that another approach would be to keep this attribute mandatory,
update the XML files, and update the documentation as well.

-- 
Joel

[-- Attachment #2: xml-tdesc.c.diff --]
[-- Type: text/plain, Size: 1046 bytes --]

Index: xml-tdesc.c
===================================================================
RCS file: /cvs/src/src/gdb/xml-tdesc.c,v
retrieving revision 1.10
diff -u -p -r1.10 xml-tdesc.c
--- xml-tdesc.c	1 Jan 2008 22:53:13 -0000	1.10
+++ xml-tdesc.c	29 Feb 2008 03:36:06 -0000
@@ -114,7 +114,14 @@ tdesc_start_target (struct gdb_xml_parse
 		    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
   struct tdesc_parsing_data *data = user_data;
-  char *version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  int length;
+  char *version;
+  
+  length = VEC_length (gdb_xml_value_s, attributes);
+  if (length == 0)
+    return;
+
+  version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
 
   if (strcmp (version, "1.0") != 0)
     gdb_xml_error (parser,
@@ -338,7 +345,7 @@ static const struct gdb_xml_element feat
 };
 
 static const struct gdb_xml_attribute target_attributes[] = {
-  { "version", GDB_XML_AF_NONE, NULL, NULL },
+  { "version", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };