Paul Eggert <eggert@cs.ucla.edu> 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.