Re: [libvirt] [PATCH 3/3] Add Memory Device Information to virSysinfoRead() from dmidecode type 17

On Tue, Jun 21, 2011 at 01:21:34PM +0900, Minoru Usui wrote: > Add Memory Device Information to virSysinfoRead() from dmidecode type 17 > > Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp&gt; > --- > src/util/sysinfo.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++- > src/util/sysinfo.h | 18 +++++ > 2 files changed, 212 insertions(+), 1 deletions(-) > > diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c > index a1eb92b..7ebf355 100644 > --- a/src/util/sysinfo.c > +++ b/src/util/sysinfo.c > @@ -74,6 +74,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def) > VIR_FREE(def->system_family); > > VIR_FREE(def->processor); > + VIR_FREE(def->memory); Again, I believe this leaks all the strings inside the struct > +static char * > +parseMemoryDeviceInfo(char *base, virSysinfoDefPtr ret) Same note about method naming with 'virSysinfo' prefix

> diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h > index f098e9d..a15c5ac 100644 > --- a/src/util/sysinfo.h > +++ b/src/util/sysinfo.h > @@ -49,6 +49,21 @@ struct _virProcessorinfoDef { > char *processor_part_number; > }; > > +typedef struct _virMemoryDeviceinfoDef virMemoryDeviceinfoDef; > +typedef virMemoryDeviceinfoDef *virMemoryDeviceinfoDefPtr; > +struct _virMemoryDeviceinfoDef { > + char *memory_size; > + char *memory_form_factor; > + char *memory_locator; > + char *memory_bank_locator; > + char *memory_type; > + char *memory_type_detail; > + char *memory_speed; > + char *memory_manufacturer; > + char *memory_serial_number; > + char *memory_part_number; > +}; Can we rename this struct to 'virSysinfoMemoryDef' Do we really want all these fields to be strings, or can some of them be stored as 'unsigned long long' perhaps for greater validation ?

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 :|