On Tue, Apr 19, 2016 at 04:55:31PM +0200, Ard Biesheuvel wrote: > We manage to use both an AND operation with 'gCacheAlignment - 1' and a > modulo operation with 'gCacheAlignment' in the same compound if statement. > Since gCacheAlignment is a global of which the compiler cannot guarantee > that it is a power of two,
Are you saying this is an "undefined behaviour" disaster waiting to happen? > simply use the AND version, and use it against > the bitwise OR of the two values to be tested. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c > b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c > index 1e6b288b10b9..834afdba10ef 100644 > --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c > +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c > @@ -92,8 +92,7 @@ DmaMap ( > > *Mapping = Map; > > - if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) || > - ((*NumberOfBytes % gCacheAlignment) != 0)) { > + if ((((UINTN)HostAddress | *NumberOfBytes) & (gCacheAlignment - 1)) != 0) { While the above is correct code, and less verbose, I am a bear of very little brain, and OR:ing an address with a counter makes it hurt. Could the above instead be (more verbose but hopefully not relevantly less efficient with a halfway decent compiler): if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) || (*NumberOfBytes & (gCacheAlignment - 1) != 0)) (It would have the benefit of making the diffstat immediately obvious.) > > // Get the cacheability of the region > Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor); > -- > 2.5.0 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
