From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1LaGtQ-0001m0-2J
	for qemu-devel@nongnu.org; Thu, 19 Feb 2009 16:57:16 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1LaGtN-0001lO-Jb
	for qemu-devel@nongnu.org; Thu, 19 Feb 2009 16:57:15 -0500
Received: from [199.232.76.173] (port=59480 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1LaGtN-0001lB-C4
	for qemu-devel@nongnu.org; Thu, 19 Feb 2009 16:57:13 -0500
Received: from mx2.redhat.com ([66.187.237.31]:53990)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <ehabkost@redhat.com>) id 1LaGtM-0007Dj-SE
	for qemu-devel@nongnu.org; Thu, 19 Feb 2009 16:57:13 -0500
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH 1/4] vmdk: check for negative sector nums also
From: Eduardo Habkost <ehabkost@raisama.net>
References: <1235078376-25559-1-git-send-email-ehabkost@redhat.com>
	<1235078376-25559-2-git-send-email-ehabkost@redhat.com>
	<499DD2C8.1050606@mail.berlios.de>
In-Reply-To: <499DD2C8.1050606@mail.berlios.de>
Date: Thu, 19 Feb 2009 18:56:41 -0300
Message-Id: <1235080301-sup-7932@blackpad>
Reply-To: qemu-devel@nongnu.org
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: Stefan Weil <weil@mail.berlios.de>
Cc: qemu-devel <qemu-devel@nongnu.org>

Excerpts from Stefan Weil's message of Qui Fev 19 18:44:40 -0300 2009:
> Eduardo Habkost schrieb:
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  block-vmdk.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/block-vmdk.c b/block-vmdk.c
> > index 71d7504..416fb95 100644
> > --- a/block-vmdk.c
> > +++ b/block-vmdk.c
> > @@ -649,7 +649,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
> >   
> 
> Why is sector_num signed? An unsigned quantity would simplify the code below
> (no need to check for < 0).

It's part of the block device interface:

    int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
                          const uint8_t *buf, int nb_sectors);

Changing it would require changing every other block driver. Shall we do
that?


> 
> >      uint64_t cluster_offset;
> >      static int cid_update = 0;
> >  
> > -    if (sector_num > bs->total_sectors) {
> > +    if (sector_num < 0 || sector_num > bs->total_sectors) {
> >          fprintf(stderr,
> >                  "(VMDK) Wrong offset: sector_num=0x%" PRIx64
> >                  " total_sectors=0x%" PRIx64 "\n",
> >   
> 
> Regards
> Stefan Weil
-- 
Eduardo