Re: [PATCH] Change module interface to no longer use GMP objects directly.

[Paul Eggert <eggert@cs.ucla.edu>]

On 11/19/19 1:12 PM, Philipp Stephani wrote:

> I'm not against a typedef to provide clearer semantics, but then we'd
> also have to define EMACS_LIMB_MAX etc., and I see relatively little
> benefit in that (since we can't realistically change it anyway later).

I can see a benefit, partly for the cleaner semantics, partly as it lets 
us arrange for Emacs limbs to be the same size as GMP limbs. If we can't 
realistically change this later, let's do it the better way the first 
time. It's easy enough to do; for example, we can define EMACS_LIMB_MAX 
to be the equivalent of ((emacs_limb_t) -1).

> The current "unsigned long" type is identical to mpz_limb_t on most
> platforms

unsigned long differs from mpz_limb_t on significant-enough platforms 
(MinGW, Cygwin, ...) that it's worth creating an emacs_limb_t to support 
those platforms better.

> I'm not feeling strongly about the function in isolation, but the
> current interface is modeled after the existing copy_string_contents

Why does copy_string_contents always return true when it returns? If 
there's no good reason, let's have the new function use a cleaner API. A 
flaw in an old function's API does not require us to have a similar flaw 
in the new one.

>> It should document that this size cannot exceed (min (PTRDIFF_MAX,
>> SIZE_MAX) / sizeof (emacs_limb_t))....
> 
> It's true that this allows
> callers to use malloc without checking for overflow, and it seems this
> already follows from the C standard (can an array with total size
> larger than SIZE_MAX exist?),
It does not follow from the C standard, as there's no requirement 
anywhere that Emacs internally must represent each integer as a 
contiguous array.

I found the guarantee useful in the first function I wrote that 
attempted to use the proposed API, so that's good evidence that it's 
worth documenting.