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

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 mxm nes nec co jp>
> ---
>  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 :|


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