Junio C Hamano <[email protected]> writes:
> Vicent Marti <[email protected]> writes:
>
>> The library is re-licensed under the GPLv2 with the permission of Daniel
>> Lemire, the original author. The source code for the C version can
>> be found on GitHub:
>>
>> https://github.com/vmg/libewok
>>
>> The original Java implementation can also be found on GitHub:
>>
>> https://github.com/lemire/javaewah
>> ---
>
> Please make sure that all patches are properly signed off.
>
>> Makefile | 6 +
>> ewah/bitmap.c | 229 +++++++++++++++++
>> ewah/ewah_bitmap.c | 703
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> ewah/ewah_io.c | 199 +++++++++++++++
>> ewah/ewah_rlw.c | 124 +++++++++
>> ewah/ewok.h | 194 +++++++++++++++
>> ewah/ewok_rlw.h | 114 +++++++++
>
> This is lovely. A few comments after an initial quick scan-through.
>
> - The code and the headers are well commented, which is good.
>
> - What's __builtin_popcountll() doing there in a presumably generic
> codepath?
>
> - Two variants of "bitmap" are given different and easy to
> understand type names (vanilla one is "bitmap", the clever one is
> "ewah_bitmap"), but at many places, a pointer to ewah_bitmap is
> simply called "bitmap" or "bitmap_i" without "ewah" anywhere,
> which waas confusing to read. Especially, the "NAND" operation
> for bitmap takes two bitmaps, while "OR" takes one bitmap and
> ewah_bitmap. That is fine as long as the combination is
> convenient for callers, but I wished the ewah variables be called
> with "ewah" somewhere in their names.
>
> - I compile with "-Werror -Wdeclaration-after-statement"; some
> places seem to trigger it.
>
> - Some "extern" declarations in *.c sources were irritating;
> shouldn't they be declared in *.h file and included?
>
> - There are some instances of "if (condition) stmt;" on a single
> line; looked irritating.
>
> - "bool" is not a C type we use (and not a particularly good type
> in C++, either).
One more.
- Use of unnecessary float (e.g. "oldval *= 1.5") were moderately
annoying.
> That is it for now. I am looking forward to read through the users
> of the library ;-)
>
> Thanks for working on this.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
