[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] 2/4 Add conf parsing and freeing



On Thu, Oct 21, 2010 at 03:45:40PM -0600, Eric Blake wrote:
> On 10/21/2010 02:16 PM, Daniel Veillard wrote:
> >  This lacks the saving of the smbios data, should not be hard really,
> >and the parsing is rather trivial, the data structures follow the XML
> >format:
> >
> >Daniel
> >
> >
> >+static void virSmbiosEntriesFree(virSmbiosEntryPtr cur)
> 
> Should this function be added to the list of useless_free_options in cfg.mk?
> 
> >+{
> >+    virSmbiosEntryPtr next;
> >+
> >+    if (cur == NULL)
> >+        return;
> 
> Redundant, given that
> 
> >+
> >+    while (cur != NULL) {
> 
> this safely skips a NULL argument.

  ah, true :-) fixed

> >@@ -3341,6 +3385,150 @@ error:
> >      goto cleanup;
> >  }
> >
> >+static virSmbiosEntryPtr
> >+virSmbiosEntryParseXML(xmlXPathContextPtr ctxt)
> >+{
> >+    char *name, *value;
> >+    virSmbiosEntryPtr def;
> >+
> >+    name = virXPathString("string(./@name)", ctxt);
> >+    if (name == NULL) {
> >+        virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> >+                  _("XML element 'entry' requires a 'name' attrbute"));
> 
> s/attrbute/attribute/

  heh,  good spotting 

[...]
> >+    if ((type<  0) || (type>  32)) {
> >+        virDomainReportError(VIR_ERR_XML_ERROR,
> >+              _("XML 'type' attribute on 'table' out of 0..32 range got %ld"),
> >+                type);
> >+        return(NULL);
> >+    }
> 
> Should you also be checking for duplicate types, or is it okay to do:
> 
>   <smbios>
>     <table type="0">
>       <entry name="Vendor">QEmu/KVM</entry>
>     </table>
>     <table type="0">
>       <entry name="Version">0.13</entry>
>     </table>
>   </smbios>

  For type 0 and 1 no they are unique in theory, but other tables
can have duplicates like type 4 since there is one per processor.
I don't think we should go too deep trying to assert semantic at
that level, since anyway the drivers will have a very restricted
view of the whole thing, and will only pick some of the information
they can process. Still I wanted the format to be as generic as
possible,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]