From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MMITt-0001jz-2r
	for qemu-devel@nongnu.org; Thu, 02 Jul 2009 05:21:25 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MMITo-0001fi-Pz
	for qemu-devel@nongnu.org; Thu, 02 Jul 2009 05:21:24 -0400
Received: from [199.232.76.173] (port=53342 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MMITo-0001fR-Fs
	for qemu-devel@nongnu.org; Thu, 02 Jul 2009 05:21:20 -0400
Received: from mx2.redhat.com ([66.187.237.31]:47404)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kwolf@redhat.com>) id 1MMITo-0004iV-4k
	for qemu-devel@nongnu.org; Thu, 02 Jul 2009 05:21:20 -0400
Message-ID: <4A4C7BC9.9050800@redhat.com>
Date: Thu, 02 Jul 2009 11:20:09 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-io: better input validation for
	vector-based commands
References: <20090701112252.GB10455@lst.de>
In-Reply-To: <20090701112252.GB10455@lst.de>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Christoph Hellwig <hch@lst.de>
Cc: qemu-devel@nongnu.org

Christoph Hellwig schrieb:
> Fix up a couple of issues with validating the input of the various
> length arguments for the vectored I/O commands:
> 
>  - do the alignment check on each length instead the always 0 count
>    argument
>  - use a long long varibale for the cvtnum return value so that we
>    can check wether it wasn't a number
>  - check for a too large argument instead of truncating it
> 
> Also refactor it into a common helper for all four calers and avoid
> parsing the numbers twice.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: qemu/qemu-io.c
> ===================================================================
> --- qemu.orig/qemu-io.c	2009-07-01 11:15:39.429264493 +0200
> +++ qemu/qemu-io.c	2009-07-01 11:24:14.463366645 +0200
> @@ -98,6 +98,56 @@ print_report(const char *op, struct time
>  	}
>  }
>  
> +/*
> + * Parse multiple length statements for vectored I/O.
> + */
> +static void *
> +parse_multiple_len(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)

Hey, a function with a comment! :-)

Now what about using this comment to describe what the function is
actually doing? I mean it doesn't only parse the lengths but prepares a
buffer and an IO vector. It even returns the buffer (before looking at
the code I wondered what void* a pure parsing function might return...)

Otherwise the patch looks good to me.

Kevin