WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk f

To: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file
From: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Date: Fri, 21 Jan 2011 12:17:58 +0000
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 21 Jan 2011 04:18:52 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <AANLkTi=1b+amdCA4A3AKxAnR6C65xv2+O3BNbGsp-T7i@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <AANLkTimSUym0u+SNm0AvNp3AZQQFspetAaXmNShkPJd4@xxxxxxxxxxxxxx> <1294995912.8240.86.camel@xxxxxxxxxxxxxxxxxxxxxx> <AANLkTikfUHHc+-gVkgnRJc722wObLF3TumpK5WSfJVAE@xxxxxxxxxxxxxx> <1295024348.12018.222.camel@xxxxxxxxxxxxxxxxxxxxxx> <AANLkTikf3TVJwE_N-OyuS-UhyA8+cgzAG__9hz3AETeM@xxxxxxxxxxxxxx> <AANLkTinJ=PYsC6vbPvU8g2T8NmyohLa=4rd9zfhTMCCO@xxxxxxxxxxxxxx> <AANLkTin1AGxH26158mn37_Oar1PgSSJoJOnGHs+XnxsV@xxxxxxxxxxxxxx> <1295532296.12018.337.camel@xxxxxxxxxxxxxxxxxxxxxx> <19768.22912.878633.622270@xxxxxxxxxxxxxxxxxxxxxxxx> <AANLkTinnrmAxaOVu1fN2qt4N2t-EzpF31sKE-Jpn-y2f@xxxxxxxxxxxxxx> <AANLkTi=1b+amdCA4A3AKxAnR6C65xv2+O3BNbGsp-T7i@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Kamala Narasimhan writes ("Re: [Xen-devel] [PATCH] xl: Perform minimal 
validation of virtual disk file while parsing config file"):
> Here is a revised patch.  Please let me know if there are further suggestions.
...
> +    assert(file_name);

I don't think we need this.  If the pointer is null dereferencing it
will crash cleanly in just a moment.

> +    if ( (strlen(file_name) == 0) && (disk_type == PHYSTYPE_PHY) )
> +        return 0;

strlen still seems overkill.

> +    if ( stat(file_name, &stat_buf) != 0 ) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Stat on virtual disk %s
> returned error - \"%s\".\n",

It is not conventional to capitalise the names of syscalls (or other
case-sensitive identifiers) even in messages, so "stat".

> +            file_name, strerror(errno));

Don't mess about with strerror yourself; use LIBXL__LOG_ERRNO.

> +    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
> +    if ( rc != 0 )
> +        return rc;

Convention in libxl is to write
    if (rc)
        return rc;

See for example libxl__fill_dom0_memory_info, which has
    if (rc)
        goto out;

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>