Am Friday 14 September 2012 schrieb Niels Möller:
> Tim Ruehsen <[email protected]> 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
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
