Re: [PATCH] Reimplement module functions

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

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

A couple of minor things I noticed in the recently-installed patch:

> const short minargs = function->min_arity;

In theory at least the arity might be greater than 32767, so this should be 
ptrdiff_t. Also, we typically don't use 'const' on locals, as it's not worth the 
screen real estate -- it should be easy even for a human reader to tell whether 
a local is assigned to later. (Likewise for 'register'.)

While looking into arity range I noticed a couple of other glitches in the 
neighborhood, and so installed the attached.

[-- Attachment #2: 0001-Minor-fixes-for-arity-ranges-in-emacs-modules.txt --]
[-- Type: text/plain, Size: 2966 bytes --]

From 848c90e3d43ed7baebab5f2d02d0a9601c6a142b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 20 May 2017 13:43:19 -0700
Subject: [PATCH] Minor fixes for arity ranges in emacs modules
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/emacs-module.c (module_make_function):
Check that arities fit into fixnums, for func-arity’s benefit.
(funcall_module): Avoid unnecessary conversion to EMACS_INT.
(module_function_arity): Allow arities greater than SHRT_MAX.
---
 src/emacs-module.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 99be4a7..5ab6913 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -358,8 +358,9 @@ module_make_function (emacs_env *env, ptrdiff_t min_arity, ptrdiff_t max_arity,
 
   if (! (0 <= min_arity
 	 && (max_arity < 0
-	     ? max_arity == emacs_variadic_function
-	     : min_arity <= max_arity)))
+	     ? (min_arity <= MOST_POSITIVE_FIXNUM
+		&& max_arity == emacs_variadic_function)
+	     : min_arity <= max_arity && max_arity <= MOST_POSITIVE_FIXNUM)))
     xsignal2 (Qinvalid_arity, make_number (min_arity), make_number (max_arity));
 
   struct Lisp_Module_Function *envptr = allocate_module_function ();
@@ -646,12 +647,11 @@ Lisp_Object
 funcall_module (const struct Lisp_Module_Function *const envptr,
                 ptrdiff_t nargs, Lisp_Object *arglist)
 {
-  EMACS_INT len = nargs;
   eassume (0 <= envptr->min_arity);
-  if (! (envptr->min_arity <= len
-	 && len <= (envptr->max_arity < 0 ? PTRDIFF_MAX : envptr->max_arity)))
+  if (! (envptr->min_arity <= nargs
+	 && (envptr->max_arity < 0 || nargs <= envptr->max_arity)))
     xsignal2 (Qwrong_number_of_arguments, module_format_fun_env (envptr),
-	      make_number (len));
+	      make_number (nargs));
 
   emacs_env pub;
   struct emacs_env_private priv;
@@ -663,12 +663,12 @@ funcall_module (const struct Lisp_Module_Function *const envptr,
     args = (emacs_value *) arglist;
   else
     {
-      args = SAFE_ALLOCA (len * sizeof *args);
-      for (ptrdiff_t i = 0; i < len; i++)
+      args = SAFE_ALLOCA (nargs * sizeof *args);
+      for (ptrdiff_t i = 0; i < nargs; i++)
 	args[i] = lisp_to_value (arglist[i]);
     }
 
-  emacs_value ret = envptr->subr (&pub, len, args, envptr->data);
+  emacs_value ret = envptr->subr (&pub, nargs, args, envptr->data);
   SAFE_FREE ();
 
   eassert (&priv == pub.private_members);
@@ -700,8 +700,8 @@ funcall_module (const struct Lisp_Module_Function *const envptr,
 Lisp_Object
 module_function_arity (const struct Lisp_Module_Function *const function)
 {
-  const short minargs = function->min_arity;
-  const short maxargs = function->max_arity;
+  ptrdiff_t minargs = function->min_arity;
+  ptrdiff_t maxargs = function->max_arity;
   return Fcons (make_number (minargs),
 		maxargs == MANY ? Qmany : make_number (maxargs));
 }
-- 
2.7.4