>>>> "Gunnar" == Gunnar Evermann
<ge204(a)eng.cam.ac.uk> writes:
Gunnar> I think this patch is wrong. Sorry for not reviewing it
Gunnar> earlier.
Yeah, well, it'll get fixed before release. Thanks!
Gunnar> - a variable (say, inhibit_autoloads) is declared in
Gunnar> emacs.c directly in C with 'int inhibit_autoloads;' no
Gunnar> DEFVAR macros involved!
Huh? emacs.c, line 3393. The DEFVAR isn't an issue because of the
initialization; it's a problem because it imports the variable to pdump
land. This allows the pdumper to overwrite the user's command line
settings.
Gunnar> note it doesn't get initialised to anything.
It's a global. I thought globals were guaranteed to be initialized to
zero---that's what `Internals | Rules When Writing New C Code | Adding
Global Lisp Variables' says. Is that incorrect?
Maybe it's stylistically bad, but technically it should work, right?
Gunnar> In particular all this breaks for values which get set to
Gunnar> 1 by default in vars_of_emacs(), like
Gunnar> inhibit_site_[lisp|modules] (at least depending on
Gunnar> configure time constants).
This is incorrect implementation of configure constants if the
variable is settable from the command line, I believe. I should have
caught and fixed that. (Et tu, Olivier! ;-)
+ Note that if the variable is never DEFVAR'd, saving/restoring
is not
+ needed.
Gunnar> I assume that this last sentence is meant to refer to
Gunnar> INHIBIT_SITE_LISP and _MODULES? They are always
Gunnar> DEFVAR'd. It is the *initialisation* to 1 that is
Gunnar> conditional.
No, it's general advice to anybody who implements a command argument
in the future, and means what it says. I didn't mean to say that if
an arbitrary variable _is_ DEFVAR'd it _must_ be saved/restored; that
is obviously false---(p)dumping couldn't work at all. I meant that
command arguments that are not DEFVAR'd can't be stomped, and don't
need saving. I will improve that comment immediately (and the rest of
the patch pending discussion ;).
It's clear I did the wrong thing in conditionalizing the saves. For
variables that are settable by command line option, the saves/restores
should be unconditional, and the conditional initialization should be
moved to the declaration.
I would argue that initialization of a variable settable by command
line option must not be done in vars_of_*, and it should be done at
the declaration. At least for these pdump-affected variables.
Gunnar> Finally I don't see a way to affect inhibit_site_lisp by
Gunnar> command line options anyway, thus making the whole
Gunnar> save&restore thing overkill.
That is correct.
I don't remember accurately what I was thinking at the time, but I
guess that I traced inhibit_autoloads carefully, and then assumed the
others were all was analogous to it. That assumption is false, as you
point out.
--
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Institute of Policy and Planning Sciences Tel/fax: +81 (298) 53-5091
_________________ _________________ _________________ _________________
What are those straight lines for? "XEmacs rules."