Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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