From: Paul Eggert <eggert@cs.ucla.edu>
To: Philipp Stephani <p.stephani2@gmail.com>, emacs-devel@gnu.org
Cc: Philipp Stephani <phst@google.com>
Subject: Re: [PATCH] Reimplement module functions
Date: Sat, 20 May 2017 13:46:20 -0700 [thread overview]
Message-ID: <5057be0b-456d-e7d9-df36-7dc52e4b540c@cs.ucla.edu> (raw)
In-Reply-To: <CAArVCkQHECW4=69e0gUOUwqk4DhHVKFcZnnnCpq826sNeRu_uw@mail.gmail.com>
[-- 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
next prev parent reply other threads:[~2017-05-20 20:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-13 14:44 [PATCH] Reimplement module functions Philipp Stephani
2017-05-13 17:05 ` Eli Zaretskii
2017-05-14 3:07 ` Noam Postavsky
2017-05-14 14:11 ` Eli Zaretskii
2017-05-14 11:40 ` Aurélien Aptel
2017-05-14 14:30 ` Eli Zaretskii
2017-05-14 18:08 ` Philipp Stephani
2017-05-14 18:09 ` Philipp Stephani
2017-05-20 13:37 ` Philipp Stephani
2017-05-20 20:46 ` Paul Eggert [this message]
2017-05-21 20:20 ` Philipp Stephani
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5057be0b-456d-e7d9-df36-7dc52e4b540c@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=emacs-devel@gnu.org \
--cc=p.stephani2@gmail.com \
--cc=phst@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).