From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gianni Tedesco <gianni.tedesco@citrix.com>
Subject: Re: [PATCH] xl: Perform minimal validation of virtual
	disk file while parsing config file
Date: Thu, 20 Jan 2011 14:12:49 +0000
Message-ID: <1295532769.12018.339.camel@qabil.uk.xensource.com>
References: <AANLkTimSUym0u+SNm0AvNp3AZQQFspetAaXmNShkPJd4@mail.gmail.com>
	<1294995912.8240.86.camel@zakaz.uk.xensource.com>
	<AANLkTikfUHHc+-gVkgnRJc722wObLF3TumpK5WSfJVAE@mail.gmail.com>
	<1295024348.12018.222.camel@qabil.uk.xensource.com>
	<AANLkTikf3TVJwE_N-OyuS-UhyA8+cgzAG__9hz3AETeM@mail.gmail.com>
	<AANLkTinJ=PYsC6vbPvU8g2T8NmyohLa=4rd9zfhTMCCO@mail.gmail.com>
	<AANLkTin1AGxH26158mn37_Oar1PgSSJoJOnGHs+XnxsV@mail.gmail.com>
	<1295532296.12018.337.camel@qabil.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <1295532296.12018.337.camel@qabil.uk.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Kamala Narasimhan <kamala.narasimhan@gmail.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Thu, 2011-01-20 at 14:04 +0000, Gianni Tedesco wrote:
> On Wed, 2011-01-19 at 18:26 +0000, Kamala Narasimhan wrote:
> > Apologies.  I inadvertently neglected Gianni's suggestion to switch to
> > logging from fprintf.
> > 
> > Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>
> > 
> > Kamala
> > 
> > diff -r fe8a177ae9cb tools/libxl/libxl.c
> > --- a/tools/libxl/libxl.c       Wed Jan 19 15:29:04 2011 +0000
> > +++ b/tools/libxl/libxl.c       Wed Jan 19 13:23:16 2011 -0500
> > @@ -826,6 +826,41 @@ skip_autopass:
> > 
> >  /******************************************************************************/
> > 
> > +static int validate_virtual_disk(libxl_ctx *ctx, char *file_name,
> > libxl_disk_phystype disk_type)
> > +{
> > +    struct stat stat_buf;
> > +
> > +    if ( file_name == NULL ) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk file name is NULL!\n");
> > +        return 0;
> > +    }
> 
> I prefer assert() for things caused by programmer error. But in this
> case we could just let the dereference below catch it...
> 
> > +    /* Return without further validation for empty cdrom drive.
> > +       Note: Post 4.1 we need to change the interface to handle empty
> > +       cdrom rather than go with the below assumption.
> > +     */
> 
> So this handles CD-ROM images too? See below...
> In which case libxl_cdrom_insert() needs the same addition?

Ah, my mistake, libxl_cdrom_insert() is implemented in terms of
libxl_device_disk_add() so only one check is necessary.

Gianni