Message97211
> A few comments on coding style:
Thank you for your remarks. I will update the patch accordingly.
> * make sure that the name of a symbol matches the value, e.g.
>
> #define LONG_BITMASK (LONG_BIT-1)
> #define BLOOM(mask, ch) ((mask & (1 << ((ch) & LONG_BITMASK))))
>
> LONG_BITMASK has a value of 0x1f (31) - that's a single byte, not
> a long value. In this case, 0x1f is an implementation detail of
> the simplified Bloom filter used for set membership tests in the
> Unicode implementation.
>
> When adjusting the value to be platform dependent, please check
> that the implementation does work for platforms that have
> more than 31 bits available for (signed) longs.
>
> Note that you don't need to expose that value separately if
> you stick to using BLOOM() directly.
Since the same value is used to build the mask, I assume it's better to keep the value around (or use (LONG_BIT-1) directly?).
mask |= (1 << (ptr[i] & LONG_BITMASK));
s/LONG_BITMASK/BLOOM_BITMASK/ is not confusing? |
|
Date |
User |
Action |
Args |
2010-01-04 13:54:53 | flox | set | recipients:
+ flox, lemburg, pitrou, eric.smith, ezio.melotti |
2010-01-04 13:54:53 | flox | set | messageid: <1262613293.61.0.409210565654.issue7622@psf.upfronthosting.co.za> |
2010-01-04 13:54:52 | flox | link | issue7622 messages |
2010-01-04 13:54:51 | flox | create | |
|