|
From: | Paul Eggert |
Subject: | Re: [PATCH] Change module interface to no longer use GMP objects directly. |
Date: | Tue, 19 Nov 2019 13:54:25 -0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
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 does not follow from the C standard, as there's no requirement anywhere that Emacs internally must represent each integer as a contiguous array.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?),
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.
[Prev in Thread] | Current Thread | [Next in Thread] |