Florent Xicluna <la...@yahoo.fr> added the comment:
> 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?
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue7622>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
