Patrick Steinhardt <ps@xxxxxx> writes:
>> + for (int i = 1; i < ARRAY_SIZE(hash_algos); i++) {
>
> s/int/size_t/
If ARRAY_SIZE(hash_algos) is an unbounded quantity that is
externally controlled, this does make very much sense, but for
hash_algos[]? It is not worth the patch noise to go and fix it.
>> +#define TEST_HASH_STR(data, expected_sha1, expected_sha256) \
>> + { \
>
> These macros should like start with `do {`. The reason why we do this is
> that the compiler will complain if there is no semicolon after the macro.
The idiom is
#define foo(a,b,c) do { \
...; \
} while (0)
so that you can write
foo(1,2,3);
as if it is a regular function call terminated _with_ a semicolon.
Also this allows us to say
if (condition)
foo(1,2,3);
else
foo(4,5,6);
which would break if the definition were a mere
#define foo(x,y,z) { \
... \
}
in which case ";" after the first foo() terminates the "if" statement
and "else" triggers a syntax error.
>> + TEST_HASH_STR(
>> + "", "da39a3ee5e6b4b0d3255bfef95601890afd80709",
>> + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855");
>
> I think these might've been a bit easier to read if they formatted like
> this:
>
> TEST_HASH_STR("",
> "da39a3ee5e6b4b0d3255bfef95601890afd80709",
> "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855");
>
> TEST_HASH_STR("a",
> "86f7e437faa5a7fce15d1ddcb9eaeaea377667b8",
> "ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb");
Oh, absolutely.
