emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Dynamic modules: emacs-module.c and signaling errors


From: Philipp Stephani
Subject: Re: Dynamic modules: emacs-module.c and signaling errors
Date: Wed, 25 Nov 2015 18:09:25 +0000



Eli Zaretskii <address@hidden> schrieb am Di., 24. Nov. 2015 um 20:41 Uhr:
In "emacs -Q", load the modules/mod-test/mod-test module, then try
this:

  M-: (mod-test-sum "1" 2) RET

Result: Emacs aborts.  This happens because the eassert in
module_extract_integer aborts, when that function is called for the
2nd time:

  static intmax_t
  module_extract_integer (emacs_env *env, emacs_value n)
  {
    check_main_thread ();
    eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
    Lisp_Object l = value_to_lisp (n);
    if (! INTEGERP (l))
      {
        module_wrong_type (env, Qintegerp, l);
        return 0;
      }

The first call to module_extract_integer correctly detects the wrong
type of argument and calls module_wrong_type.  But module_wrong_type
just records the problem in the env structure, it doesn't signal any
Lisp error, like an Emacs primitive would.  So the actual error goes
undetected, and is masked by the assertion violation (because Emacs is
built with --enable-checking).

Since this obviously works as it was designed, my question is: how
should a module be written so that this kind of errors signal a normal
Lisp error we are accustomed with Emacs primitives?


I still need to fix most of the example module (unfortunately that was written before the error handling got implemented), but the gist is that functions returning emacs_value should usually be called like this:

emacs_value ret = env->foo(env, ...);
if (ret == NULL) return NULL;

This will cause errors to be reported back to Emacs, where they are thrown as normal signals.

I have thought about how we can prevent some accidental misuse. I propose that all environment functions should have a signature like this:

__attribute__((warn_unused_result))
bool (*func)(emacs_env *env, ..., result_type* result);

instead of returning result directly. This way users get a warning if they forget to check for errors. The above example would then turn into:

emacs_value ret;
if (!env->foo(env, ..., &ret)) return false;

which isn't worse, but avoids sentinel values and makes the API harder to misuse. Thoughts?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]