Re: [libvirt] [PATCH 1/3] Cleanup virSysinfoRead()

On Tue, Jun 21, 2011 at 01:19:17PM +0900, Minoru Usui wrote: > [Cleanup] Separate BIOSInfo and SystemInfo part from virSysinfoRead() > > > Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp&gt; > --- > src/util/sysinfo.c | 210 ++++++++++++++++++++++++++++++++-------------------- > 1 files changed, 130 insertions(+), 80 deletions(-) > > diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c > index 40ec2e3..53122f7 100644 > --- a/src/util/sysinfo.c > +++ b/src/util/sysinfo.c > @@ -96,37 +96,10 @@ virSysinfoRead(void) { > > #else /* !WIN32 */ > > -virSysinfoDefPtr > -virSysinfoRead(void) { > - char *path, *cur, *eol, *base; > - virSysinfoDefPtr ret = NULL; > - char *outbuf = NULL; > - virCommandPtr cmd; > - > - path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); > - if (path == NULL) { > - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, > - _("Failed to find path for %s binary"), > - SYSINFO_SMBIOS_DECODER); > - return NULL; > - } > - > - cmd = virCommandNewArgList(path, "-q", "-t", "0,1", NULL); > - VIR_FREE(path); > - virCommandSetOutputBuffer(cmd, &outbuf); > - if (virCommandRun(cmd, NULL) < 0) { > - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, > - _("Failed to execute command %s"), > - path); > - goto cleanup; > - } > - > - if (VIR_ALLOC(ret) < 0) > - goto no_memory; > - > - ret->type = VIR_SYSINFO_SMBIOS; > - > - base = outbuf; > +static char * > +parseBIOSInfo(char *base, virSysinfoDefPtr ret) Can you make sure this method, and all other ones added to this file have the "virSysinfo" prefix on their name

> +static char * > +parseSystemInfo(char *base, virSysinfoDefPtr ret) > +{ > +static void > +BIOSInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf) > +static void > +SystemInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf) > +{ > +/** > + * virSysinfoFormat: > + * @def: structure to convert to xml string > + * @prefix: string to prefix before each line of xml > + * > + * This returns the XML description of the sysinfo, or NULL after > + * generating an error message. > + */ > +char * > +virSysinfoFormat(virSysinfoDefPtr def, const char *prefix) > +{ > + const char *type = virSysinfoTypeToString(def->type); > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + > + if (!type) { > + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, > + _("unexpected sysinfo type model %d"), > + def->type); > + return NULL; > } > > + virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n", prefix, type); > + > + BIOSInfoFormat(def, prefix, &buf); > + SystemInfoFormat(def, prefix, &buf); > + > virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix); > > return virBufferContentAndReset(&buf); An existing bug here that needs fixing. There should be a call to if (virBufferError(&buf)) { virReportOOMError(); return NULL; } just before the virBufferContentAndReset() usage.

Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|