Re: [PATCH 2/2] libxl: zero the vtpm structures on allocation

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 04/12/2013 10:10 AM, Ian Campbell wrote:
> On Thu, 2013-03-14 at 14:23 +0000, Daniel De Graaf wrote:
>> This avoids returning unallocated memory in the libxl_device_vtpm
>> structure since libxl_device_vtpm_init is not called here.
>
> Does the code not either loop over ndirs and initialise every one or
> alternatively fail and cleanup without returning the uninitialised
> memory?

The code used to initialize every member and so did not call the _init
function for vtpms; adding a new member to the structure causes this
to miss the new member. An alternative to this patch is to add a call
to libxl_device_vtpm_init inside the loop, similar the disk device type.
The nic device just does memset(nic, 0, sizeof(*nic)), like this patch.

> If this is an issue it expect it is an issue for libxl_device_*_list?
> Probably they should all be calling their respective init functions,
> perhaps via a new libxl_..._init_array helper?

At least disk and nic handle this on a per-element basis, so there's no
immediate advantage to an array helper.

>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Cc:	Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc:	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc:	Ian Campbell <ian.campbell@citrix.com>
>> ---
>>   tools/libxl/libxl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index e442afc..2b64700 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1853,7 +1853,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *n
>>       fe_path = libxl__sprintf(gc, "%s/device/vtpm", libxl__xs_get_dompath(gc, domid));
>>       dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
>>       if(dir) {
>> -       vtpms = malloc(sizeof(*vtpms) * ndirs);
>> +       vtpms = calloc(sizeof(*vtpms), ndirs);
>>          libxl_device_vtpm* vtpm;
>>          libxl_device_vtpm* end = vtpms + ndirs;
>>          for(vtpm = vtpms; vtpm < end; ++vtpm, ++dir) {
>
>


-- 
Daniel De Graaf
National Security Agency