RE: [PATCH] imsm: fix: does not allow to use invalid chunk size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: NeilBrown [mailto:neilb@xxxxxxx]
> Sent: Tuesday, December 06, 2011 1:56 AM
> To: Hawrylewicz Czarnowski, Przemyslaw
> Cc: linux-raid@xxxxxxxxxxxxxxx; Williams, Dan J; Labun, Marcin;
> Ciechanowski, Ed
> Subject: Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
> 
> On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski
> <przemyslaw.hawrylewicz.czarnowski@xxxxxxxxx> wrote:
> 
> > Only least significant bit of chunk size provided by user has been
> > used in test with OROM capabilities. This way user could pass value
> > which is not a power of 2.
> >
> > Signed-off-by: Przemyslaw Czarnowski
> > <przemyslaw.hawrylewicz.czarnowski@xxxxxxxxx>
> > ---
> >  platform-intel.h |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/platform-intel.h b/platform-intel.h index
> > 6c094d7..99450ba 100644
> > --- a/platform-intel.h
> > +++ b/platform-intel.h
> > @@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const
> > struct imsm_orom *orom)  static inline int imsm_orom_has_chunk(const
> > struct imsm_orom *orom, int chunk)  {
> >  	int fs = ffs(chunk);
> > +	int orom_chunk_bit;
> >
> >  	if (!fs)
> >  		return 0;
> >  	fs--; /* bit num to bit index */
> > -	return !!(orom->sss & (1 << (fs - 1)));
> > +	orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
> > +	return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
> >  }
> >
> >
> 
> applied, thanks.
> 
> However it feels a bit clumsy, though maybe that is just me.
> I would use a separate explicit test for "is a power of 2".
> ie.
> 
>   if (chunk & (chunk-1))
>         return 0; /* not a power of 2 */
>   fs = ffs(chunk);
>   return !!(orom->sss & (1 << (fs-2)));
> 
You're right,

Please get the one from the bottom of this email then... It is based on your input (~90%) so sign it up and ship:)
Moreover previous one is not the right one I wanted to send out...

--
Best Regards,
Przemyslaw Hawrylewicz-Czarnowski

> or something like that.
> 
> NeilBrown

-- 
Subject: [PATCH] fix: imsm: validate strip size - tuned up

Neil's proposal seems more reasonable and shows what is really going on
here.

Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@xxxxxxxxx>
---
 platform-intel.h |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/platform-intel.h b/platform-intel.h
index 99450ba..c997f1b 100644
--- a/platform-intel.h
+++ b/platform-intel.h
@@ -124,13 +124,12 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
 static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
 {
        int fs = ffs(chunk);
-       int orom_chunk_bit;
-
        if (!fs)
                return 0;
        fs--; /* bit num to bit index */
-       orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
-       return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
+       if (chunk & (chunk-1))
+               return 0; /* not a power of 2 */
+       return !!(orom->sss & (1 << (fs - 1)));
 }
 
 
-- 
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux