Am Friday 14 September 2012 schrieb Niels Möller:
Tim Ruehsen tim.ruehsen@gmx.de writes:
But after all, you still won't need them. Just have a look in the patched hmac-test.c, where you can find are mixtures of hex data and strings.
I haven't read the complete patch very carefully. In hmac-test.c, you have introduced a flag argument, which is not as clumsy as I had expected (but I prefer my preprocessor hack before your algorithm enum and corresponding switch statement; I think it's clear that we have somewhat different taste).
I just saw how the _NETTLE_HASH macro is working. Using that, the enum could be removed. E.g. by including md5-meta.h, we have a nettle_md5 variable declared, which holds all we need (context, init, update, maybe not a set_key function). At least something like that.
In other places, you have introduced additional functions, like test_hash and test_hash_hex. I prefer a style which looks more like functional programming, with a single test_hash using a canonical representation of the input, and then the caller invokes any helper macro or function needed to convert data from whatever form is most convenient in the test file.
Ok, the proposed HL() function would obsolete test_hash_hex().
Then I happily admit that the current implementation with macros is not very beautiful.
:-)
/* example test function with testdata_t */ void test_cipher(const struct nettle_cipher *cipher,
testdata_t *key, testdata_t *cleartext, testdata_t *ciphertext){
// use key.length instead of key_length, etc. ...
free(ciphertext); free(cleartext); free(key);
}
In this model, we have a producer/consumer convention. The passed in data is *always* malloced, to be freed by the test function. That's a good idea, if we now consider deallocation important, that convention will simplify things (at the cost of an unnecessary allocation in the few cases where we now pass a string literal as the pointer).
Not the allocation/strdup in unnecessary. It is just the call to decode_hex_length().
xmalloc should be slightly changed to not allow size 0 (either fail or allocated a 1 byte size). From my man pages: If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free().
I don't see the problem.
First, it is important to note that callers of xalloc are not expected to check the return value of xalloc, it will never return NULL except maybe when size == 0, and in that case it does *not* indicate that allocation failed. By definition, if xalloc returned, then it did not fail.
Noow, xalloc is sometimes called with size 0, for example if you do decode_hex_dup(""). That's why its check for malloc failing also has to check size. It will succeed (i.e., not abort) no matter which convention for size == 0 which malloc follows, and simply return whatever malloc returns.
As far as I see, it doesn't matter if malloc (and hence xalloc) returns NULL or a valid pointer, in either case, the caller should not dereference the pointer, and eventually pass it to on to free.
My proposed HL() routine expects xalloc never to return NULL. But that is easy to check.
The only argument for a change is, that malloc and thus xalloc behaves different on different plattforms. A defined behaviour of xalloc for all plattforms would make it more predictable. It is just a practical consideration for testing. Does not really matter to me.
Regards, Tim