Message ID | 1274968015-23599-1-git-send-email-Jes.Sorensen@redhat.com |
---|---|
State | New |
Headers | show |
Am 27.05.2010 15:46, schrieb Jes.Sorensen@redhat.com: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > C defaults to int, so make definition of BDRV_SECTOR_SIZE 64 bit > safe as it and BDRV_SECTOR_MASK may be used against 64 bit addresses. > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> Thanks, applied to the block branch. Kevin
On 05/27/2010 04:27 PM, Kevin Wolf wrote: > Am 27.05.2010 15:46, schrieb Jes.Sorensen@redhat.com: >> From: Jes Sorensen<Jes.Sorensen@redhat.com> >> >> C defaults to int, so make definition of BDRV_SECTOR_SIZE 64 bit >> safe as it and BDRV_SECTOR_MASK may be used against 64 bit addresses. >> >> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com> > > Thanks, applied to the block branch. Candidate for stable too? Paolo
On 05/27/10 17:38, Paolo Bonzini wrote: > On 05/27/2010 04:27 PM, Kevin Wolf wrote: >> Am 27.05.2010 15:46, schrieb Jes.Sorensen@redhat.com: >>> From: Jes Sorensen<Jes.Sorensen@redhat.com> >>> >>> C defaults to int, so make definition of BDRV_SECTOR_SIZE 64 bit >>> safe as it and BDRV_SECTOR_MASK may be used against 64 bit addresses. >>> >>> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com> >> >> Thanks, applied to the block branch. > > Candidate for stable too? It should be safe to apply, but I didn't find any current users where the mask was applied in a way where it was causing problems. Not sure if you want the noise, or apply it as better safe than sorry? Cheers, Jes
On 05/27/2010 05:44 PM, Jes Sorensen wrote: > > Candidate for stable too? > It should be safe to apply, but I didn't find any current users where > the mask was applied in a way where it was causing problems. Not sure if > you want the noise, or apply it as better safe than sorry? The only use in fact is this: addr = qemu_get_be64(f); flags = addr & ~BDRV_SECTOR_MASK; which is safe since the ~~ cancels to give back 511 again. So nevermind, just asking. If there are no bugs related to it it seems just as safe not to apply it. Paolo
On 05/28/10 10:32, Paolo Bonzini wrote: > On 05/27/2010 05:44 PM, Jes Sorensen wrote: >> > Candidate for stable too? >> It should be safe to apply, but I didn't find any current users where >> the mask was applied in a way where it was causing problems. Not sure if >> you want the noise, or apply it as better safe than sorry? > > The only use in fact is this: > > addr = qemu_get_be64(f); > flags = addr & ~BDRV_SECTOR_MASK; > > which is safe since the ~~ cancels to give back 511 again. So > nevermind, just asking. If there are no bugs related to it it seems > just as safe not to apply it. That is correct, which is why I don't think it is necessary for the stable release. However I want to see the fix in upstream as the macro is likely to get used for other things in the future and it's a hidden bug waiting to happen. Cheers, Jes
diff --git a/block.h b/block.h index 24efeb6..5e05612 100644 --- a/block.h +++ b/block.h @@ -38,7 +38,7 @@ typedef struct QEMUSnapshotInfo { #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) #define BDRV_SECTOR_BITS 9 -#define BDRV_SECTOR_SIZE (1 << BDRV_SECTOR_BITS) +#define BDRV_SECTOR_SIZE (1ULL << BDRV_SECTOR_BITS) #define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1) typedef enum {