Re: [Xen-devel] [PATCH 14/29] libxl: don't leak xs_read results in libxl__device_nic_from_xs_be

[Matthew Daley <mattjd@xxxxxxxxx>]

On Wed, Oct 30, 2013 at 9:58 PM, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> On 30/10/13 08:51, Matthew Daley wrote:
>> Coverity-ID: 1055866
>> Signed-off-by: Matthew Daley <mattjd@xxxxxxxxx>
>> ---
>>  tools/libxl/libxl.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 0b29f32..234f3c1 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -2975,9 +2975,10 @@ static void libxl__device_nic_from_xs_be(libxl__gc 
>> *gc,
>>
>>      tmp = xs_read(ctx->xsh, XBT_NULL,
>>                    libxl__sprintf(gc, "%s/handle", be_path), &len);
>> -    if ( tmp )
>> +    if ( tmp ) {
>>          nic->devid = atoi(tmp);
>> -    else
>> +        free(tmp);
>> +    } else
>>          nic->devid = 0;
>>
>>      /* nic->mtu = */
>> @@ -2987,6 +2988,7 @@ static void libxl__device_nic_from_xs_be(libxl__gc *gc,
>>      rc = libxl__parse_mac(tmp, nic->mac);
>>      if (rc)
>>          memset(nic->mac, 0, sizeof(nic->mac));
>> +    free(tmp);
>>
>>      nic->ip = xs_read(ctx->xsh, XBT_NULL,
>>                        libxl__sprintf(gc, "%s/ip", be_path), &len);
>
> I think the proper way of dealing with this is to use
> libxl__xs_read_checked instead of xs_read, that adds the results to the gc.
>

Indeed, that seems the better way. I'll try to provide a patch which
does this consistently across libxl. This current patch can be
considered dropped if so wanted.

- Matthew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel