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
|