Problem with the mp_set_memory_functions interface

[Niels Möller]

Any thoughts on this problem with the custom memory allocation
functions?

nisse at lysator.liu.se (Niels Möller) writes:

> I find this part of the interface,
>
> :     The REALLOCATE_FUNCTION parameter OLD_SIZE and the FREE_FUNCTION
> :  parameter SIZE are passed for convenience, but of course they can be
> :  ignored if not needed by an implementation.  The default functions
> :  using `malloc' and friends for instance don't use them.
>    
> extremely awkward, bordering to totally broken.

[...]

> mpz_get_str, with a NULL buffer argument, is specified as allocating
> space *exactly* enough for the digits and the NUL terminator.
>
> :  If STR is `NULL', the result string is allocated using the current
> :  allocation function (*note Custom Allocation::).  The block will be
> :  `strlen(str)+1' bytes, that being exactly enough for the string and
> :  null-terminator.
>
> So if mpz_get_str does the natural thing of allocating
>
>   mpz_sizeinbase (x, base) + is_negative + 1
>
> bytes, it *must* use realloc to shrink that allocation in case it turns
> out that mpz_sizeinbase returned a value which was one off (as it is
> allowed to do). Thats really really ugly, it's an overhead that is
> caused by the memory allocation interface, and which is totally
> unnecessary for almost all users.
>
> And then, looking at GMP's mpz/get_str.c, it apparently fails to
> handle this case correctly!

[...]

> we have a bug which will manifest itself if
>
> 1. An application registers its own allocation function, and the custom
>    free function really depends on the size argument being correct.
>
> 2. The application uses mpz_get_str with NULL buffer, and deallocates it
>    according to the procedure suggested by the mpz_get_str
>    documentation.
>
> 3. The application then calls mpz_get_str with a value and a base, for
>    which mpz_sizeinbase returns a value larger than the actual number of
>    digits.

I'd really prefer not to fix this bug by adding a mostly useless realloc
call to mpz_get_str. I think I'd suggest for the short term:

1. Send an message to gmp-announce, asking if there's anybody out there
   who relies on correct old-size arguments to the reallocate function
   and the free function. And if so, how painful it would be if that
   feature was removed.

2. Based on the above, we can hopefully deprecate the old-size argument,
   and always pass zero (which is what mini-gmp does, btw).

Longer term, I think it would make sense to replace this interface,
deleting the old-size argument altogether. At the same time, we could
rename mp_set_memory_functions to gmp_set_memory_functions. Maybe one
could also simplify it a bit by using a function pointer for realloc
only (realloc(NULL, size) and realloc(p, 0) could be used as substitutes
for malloc and free)?

Regards,
/Niels


-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.