[PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Andrew Cooper]

Coverity-ID: 1240235
 * filename may be NULL, at which point calling unlink() would be unwise.

Coverity-ID: 1240237
 * Initalise rc to 0 to avoid returning stack junk in the success case.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dom.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index bd21841..11c2683 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -2097,7 +2097,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
                           const char *userdata_userid)
 {
     GC_INIT(ctx);
-    int rc;
+    int rc = 0;
 
     libxl__domain_userdata_lock *lock;
     const char *filename;
@@ -2110,7 +2110,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
     }
 
     filename = libxl__userdata_path(gc, domid, userdata_userid, "d");
-    if (unlink(filename)) rc = ERROR_FAIL;
+    if (!filename || unlink(filename)) rc = ERROR_FAIL;
 
     libxl__unlock_domain_userdata(lock);
 out:
-- 
1.7.10.4

Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Wei Liu]

On Wed, Sep 24, 2014 at 12:39:45PM +0100, Andrew Cooper wrote:
> Coverity-ID: 1240235
>  * filename may be NULL, at which point calling unlink() would be unwise.
> 
> Coverity-ID: 1240237
>  * Initalise rc to 0 to avoid returning stack junk in the success case.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Thanks for fixing this up.

Wei.

Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Ian Campbell]

On Wed, 2014-09-24 at 12:39 +0100, Andrew Cooper wrote:
> Coverity-ID: 1240235
>  * filename may be NULL, at which point calling unlink() would be unwise.

Oops!

> Coverity-ID: 1240237
>  * Initalise rc to 0 to avoid returning stack junk in the success case.

Doing this right before the out: label is helpful since it helps the
compiler catch failure paths which forget to set rc.

> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_dom.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index bd21841..11c2683 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -2097,7 +2097,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
>                            const char *userdata_userid)
>  {
>      GC_INIT(ctx);
> -    int rc;
> +    int rc = 0;
>  
>      libxl__domain_userdata_lock *lock;
>      const char *filename;
> @@ -2110,7 +2110,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
>      }
>  
>      filename = libxl__userdata_path(gc, domid, userdata_userid, "d");
> -    if (unlink(filename)) rc = ERROR_FAIL;
> +    if (!filename || unlink(filename)) rc = ERROR_FAIL;
>  
>      libxl__unlock_domain_userdata(lock);
>  out:

Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Andrew Cooper]

On 24/09/14 15:24, Ian Campbell wrote:
> On Wed, 2014-09-24 at 12:39 +0100, Andrew Cooper wrote:
>> Coverity-ID: 1240235
>>  * filename may be NULL, at which point calling unlink() would be unwise.
> Oops!
>
>> Coverity-ID: 1240237
>>  * Initalise rc to 0 to avoid returning stack junk in the success case.
> Doing this right before the out: label is helpful since it helps the
> compiler catch failure paths which forget to set rc.

Good point - v2 incoming.

~Andrew

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  tools/libxl/libxl_dom.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index bd21841..11c2683 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -2097,7 +2097,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
>>                            const char *userdata_userid)
>>  {
>>      GC_INIT(ctx);
>> -    int rc;
>> +    int rc = 0;
>>  
>>      libxl__domain_userdata_lock *lock;
>>      const char *filename;
>> @@ -2110,7 +2110,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
>>      }
>>  
>>      filename = libxl__userdata_path(gc, domid, userdata_userid, "d");
>> -    if (unlink(filename)) rc = ERROR_FAIL;
>> +    if (!filename || unlink(filename)) rc = ERROR_FAIL;
>>  
>>      libxl__unlock_domain_userdata(lock);
>>  out:
>

Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()"):
> Good point - v2 incoming.

Just spotted this reply from Andrew.  I have my own version just
posted.

Ian.

Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] tools/libxl: Fix two errors with libxl_userdata_unlink()"):
> Doing this right before the out: label is helpful since it helps the
> compiler catch failure paths which forget to set rc.

Do we know why the compiler didn't spot that on this occasion ?

Ian.