Re: [Qemu-devel] [PATCH 1/4] vmdk: check for negative sector nums also

[Eduardo Habkost <ehabkost@raisama.net>]

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