Am Friday 14 September 2012 schrieb Niels Möller:
Tim Ruehsen tim.ruehsen@gmx.de writes:
There is just changed file outside testsuite/ and examples/ : buffer.c. nettle_buffer_clear() used to call realloc() with a length of 0 to wipe the allocated memory. But at least here it comes back with a valid pointer to one allocated byte (as valgrind reports). I use free() after the realloc if the returned pointer is not NULL (maybe realloc() should not be called at all !?).
This is a bit tricky. The nettle_buffer interface lets the user configure memory allocation using a single function pointer to the user's realloc function. It's not required to be compatible in any way with libc free.
My man page for realloc says
realloc() changes the size of the memory block pointed to by ptr to size bytes. [...] if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr).
My man page says (Embedded GNU C Library 2.13) If size was equal to 0, either NULL or a pointer suitable to be passed to free() is returned. CONFORMING TO C89, C99.
My change handles both cases.
If we need a workaround, the right place is in the realloc wrapppers in realloc.c.
We need a workaround since different libraries handle it differently. But simplified it is just a if (p) free(p);
int test_main(void) {
/* 128 bit keys */
- test_cipher(&nettle_aes128,
HL("0001020305060708 0A0B0C0D0F101112"),HL("506812A45F08C889 B97F5980038B8359"),H("D8F532538289EF7D 06B506A4FD5BE9C9"));
- test_cipher(&nettle_aes128,
"0001020305060708 0A0B0C0D0F101112","506812A45F08C889 B97F5980038B8359","D8F532538289EF7D 06B506A4FD5BE9C9");The reason I introduced the HL and LDATA macros was that I find it useful that the testcases can chose either hex data or raw binary data. A change requiring hex everywhare is not so attractive.
I handled that case in hmac-test.c by introducing a 'flags' param that gives a hint which of the data params is hex and which is not.
e.g. hmac_test(alg_md5, MD5_DIGEST_SIZE, HEX_NONE, "Jefe", "what do ya want for nothing?", "750c783e6ab0b503 eaa86e310a5db738");
hmac_test(alg_md5, MD5_DIGEST_SIZE, (HEX_KEY|HEX_MSG), "aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa", "dddddddddddddddd dddddddddddddddd" "dddddddddddddddd dddddddddddddddd" "dddddddddddddddd dddddddddddddddd" "dddd", "56be34521d144c88 dbb8c733f0e8b3f6");
- unsigned key_length = decode_hex_length(key_hex);
- const uint8_t *key = decode_hex_dup(key_hex);
- unsigned length = decode_hex_length(cleartext_hex);
- const uint8_t *cleartext = decode_hex_dup(cleartext_hex);
- const uint8_t *ciphertext = decode_hex_dup(ciphertext_hex);
...
- free((void *)ciphertext);
- free((void *)cleartext);
- free((void *)key);
It's unfortunate that the free prototype doesn't take a const void *, but we have to live with that. In this case, I think it's preferable to drop const from the declaration of those pointers, so there's no need for those explicit casts.
I personally prefer a macro, something like #define xfree(a) do { if (a) { free((void *)(a)); a=NULL; } } while (0)
The less intrusive changes I'll try to get to within a few days.
You could tell which points you would like to have changed, and i'll change them here. After we are done, the patch could be applied in whole. I'll do the coding and you the review...
Regards, Tim