From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: martin rudalics <rudalics@gmx.at>
Cc: 34318@debbugs.gnu.org, Andreas Schwab <schwab@linux-m68k.org>
Subject: bug#34318: 26.1.90; Strange behavior of two line message with running shell
Date: Tue, 12 Feb 2019 16:34:00 -0500 [thread overview]
Message-ID: <jwvimxog24x.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <5C5D52AC.4060501@gmx.at> (martin rudalics's message of "Fri, 08 Feb 2019 10:58:04 +0100")
> The problem is with
>
> 2018-06-03 Stefan Monnier <monnier@iro.umontreal.ca>
>
> Fix bug#30846, along with misc cleanups found along the way
>
> and in particular
>
> Don't call swap_in_symval_forwarding since the currently swapped
> binding is never one we've modified.
Indeed, good spotting. I installed the patch below which mostly reverts
this part of the commit.
> So please, pretty please review that entire patch again - maybe it
> contains additional sorts of problems.
This is a rather tricky part of the code, indeed. I reviewed it
thoroughly before installing it and yet here we are.
I re-reviewed it now and couldn't spot any further mistakes, but you
know what this means.
Stefan
diff --git a/src/data.c b/src/data.c
index 571114802a..ed6dedbe24 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1954,6 +1954,16 @@ Instead, use `add-hook' and specify t for the LOCAL argument. */)
(current_buffer,
Fcons (Fcons (variable, XCDR (blv->defcell)),
BVAR (current_buffer, local_var_alist)));
+
+ /* If the symbol forwards into a C variable, then load the binding
+ for this buffer now, to preserve the invariant that forwarded
+ variables must always hold the value corresponding to the
+ current buffer (they are swapped eagerly).
+ Otherwise, if C code modifies the variable before we load the
+ binding in, then that new value would clobber the default binding
+ the next time we unload it. See bug#34318. */
+ if (blv->fwd)
+ swap_in_symval_forwarding (sym, blv);
}
return variable;
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 0069ee84fe..f3b4262de4 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -508,4 +508,22 @@ binding-test-some-local
(bound-and-true-p data-tests-foo2)
(bound-and-true-p data-tests-foo3)))))))
+(ert-deftest data-tests-make-local-forwarded-var () ;bug#34318
+ ;; Boy, this bug is tricky to trigger. You need to:
+ ;; - call make-local-variable on a forwarded var (i.e. one that
+ ;; has a corresponding C var linked via DEFVAR_(LISP|INT|BOOL))
+ ;; - cause the C code to modify this variable from the C side of the
+ ;; forwarding, but this needs to happen before the var is accessed
+ ;; from the Lisp side and before we switch to another buffer.
+ ;; The trigger in bug#34318 doesn't exist any more because the C code has
+ ;; changes. Instead I found the trigger below.
+ (with-temp-buffer
+ (setq last-coding-system-used 'bug34318)
+ (make-local-variable 'last-coding-system-used)
+ ;; This should set last-coding-system-used to `no-conversion'.
+ (decode-coding-string "hello" nil)
+ (should (equal (list last-coding-system-used
+ (default-value 'last-coding-system-used))
+ '(no-conversion bug34318)))))
+
;;; data-tests.el ends here
next prev parent reply other threads:[~2019-02-12 21:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-04 18:52 bug#34318: 26.1.90; Strange behavior of two line message with running shell martin rudalics
2019-02-04 22:19 ` Andreas Schwab
2019-02-04 23:44 ` Glenn Morris
2019-02-05 8:36 ` martin rudalics
2019-02-05 9:05 ` Eli Zaretskii
2019-02-05 10:21 ` martin rudalics
2019-02-05 9:12 ` Andreas Schwab
2019-02-05 10:21 ` martin rudalics
2019-02-05 8:36 ` martin rudalics
2019-02-05 9:37 ` Andreas Schwab
2019-02-05 10:21 ` martin rudalics
2019-02-05 10:36 ` Eli Zaretskii
2019-02-05 11:39 ` Andreas Schwab
2019-02-05 13:25 ` Eli Zaretskii
2019-02-05 13:44 ` Andreas Schwab
2019-02-08 9:58 ` martin rudalics
2019-02-12 21:34 ` Stefan Monnier [this message]
2019-02-19 9:00 ` martin rudalics
2019-02-19 12:52 ` Stefan Monnier
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvimxog24x.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=34318@debbugs.gnu.org \
--cc=rudalics@gmx.at \
--cc=schwab@linux-m68k.org \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.