emacs-devel
[Top][All Lists]
Advanced

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

Re: emacs-module.c, eassert, and nonnull args


From: Philipp Stephani
Subject: Re: emacs-module.c, eassert, and nonnull args
Date: Mon, 05 Jun 2017 13:56:57 +0000



Paul Eggert <address@hidden> schrieb am Mo., 5. Juni 2017 um 04:48 Uhr:
Thanks for your recent improvements to emacs-module.c. One thing I noticed,
though, was that it added several easserts. However, there's a comment at the
start of emacs-module.c that says "Do NOT use 'eassert'". To play it safe for
now I removed the easserts, and thought I'd raise this on emacs-discuss.

As I understand it, emacs-module.c's use of eassert is intended for bugs in
Emacs itself, not for bugs in user-supplied modules. Although perhaps we need a
more-systematic way of issuing signals for screwups in modules, 'eassert' sounds
dicey for that as assertion failures are so drastic. Even though modules can
dump core on their own, should Emacs be on high alert and dump core merely
because a module has an invalid value? Plus, should ENABLE_CHECKING affect
module-screwup checking the same way that it affects eassert?

I think you are right, eassert is the wrong tool here. If at all, module developers can be expected to use normal release builds of Emacs, so eassert wouldn't help them.
In the attached patch I've implemented a command-line option '-module-assertions' that allows these assertions to be enabled at runtime. The option is meant to be used during development for batch jobs and sessions where crashing is OK.
(The commit doesn't contain documentation yet.)
 
Instead of using runtime checks, perhaps we should decorate emacs-module.h's
function declarations with __attribute__ ((__nonnull__ ((N)))) if argument N of
a module function is supposed to be nonnull, so that problems in this area can
(mostly) be caught statically. We could add a macro like the following to
src/emacs-module.h, after the definition of EMACS_NOEXCEPT:

   #if 3 < __GNUC__ + (3 <= __GNUC_MINOR__)
   # define EMACS_ARG_NONNULL(...) __attribute__ ((__nonnull__ ((__VA_ARGS__))))
   #else
   # define EMACS_ARG_NONNULL(...)
   #endif

and then use EMACS_ARG_NONNULL calls for function pointers whose arguments are
supposed to be nonnull.


Yes, that makes sense. Instead of the explicit version check (that might not work with other compilers), __has_attribute should be used if available. 

Attachment: 0001-Implement-module-assertions-for-users.txt
Description: Text document


reply via email to

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