* bug#65796: dynamic module non_local_exit_get overwrites exit signals @ 2023-09-06 22:52 Xinyang Chen 2023-09-07 7:07 ` Eli Zaretskii 0 siblings, 1 reply; 4+ messages in thread From: Xinyang Chen @ 2023-09-06 22:52 UTC (permalink / raw) To: 65796 [-- Attachment #1: Type: text/plain, Size: 596 bytes --] Currently `module_non_local_exit_get` returns pointers to fields in emacs_env_private: ``` if (p->pending_non_local_exit != emacs_funcall_exit_return) { *symbol = &p->non_local_exit_symbol; *data = &p->non_local_exit_data; } ``` this means that if one tries to: ``` funcall(...); non_local_exit_get(&s1, &d1); funcall(...); non_local_exit_get(&s2, &d2); non_local_exit_signal(s1, d1); ``` you would signal the second error, instead of the first error (I expected this to happen). It seems to me that `module_non_local_exit_get` should `allocate_emacs_value` instead. -Alan [-- Attachment #2: Type: text/html, Size: 855 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#65796: dynamic module non_local_exit_get overwrites exit signals 2023-09-06 22:52 bug#65796: dynamic module non_local_exit_get overwrites exit signals Xinyang Chen @ 2023-09-07 7:07 ` Eli Zaretskii 2023-09-07 10:24 ` Philipp Stephani via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 4+ messages in thread From: Eli Zaretskii @ 2023-09-07 7:07 UTC (permalink / raw) To: Xinyang Chen, Philipp Stephani, Daniel Colascione; +Cc: 65796 > From: Xinyang Chen <chenxinyang99@gmail.com> > Date: Wed, 6 Sep 2023 18:52:14 -0400 > > Currently `module_non_local_exit_get` returns pointers to fields > in emacs_env_private: > ``` > if (p->pending_non_local_exit != emacs_funcall_exit_return) > { > *symbol = &p->non_local_exit_symbol; > *data = &p->non_local_exit_data; > } > ``` > this means that if one tries to: > ``` > funcall(...); > non_local_exit_get(&s1, &d1); > funcall(...); > non_local_exit_get(&s2, &d2); > non_local_exit_signal(s1, d1); > ``` > you would signal the second error, instead of the first error (I expected > this to happen). > It seems to me that `module_non_local_exit_get` should > `allocate_emacs_value` instead. Philipp, Daniel: any comments? Btw, the non_local_exit_get function is currently not documented in the ELisp manual; should it be? ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#65796: dynamic module non_local_exit_get overwrites exit signals 2023-09-07 7:07 ` Eli Zaretskii @ 2023-09-07 10:24 ` Philipp Stephani via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-09-07 10:55 ` Eli Zaretskii 0 siblings, 1 reply; 4+ messages in thread From: Philipp Stephani via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-07 10:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Xinyang Chen, p.stephani2, Daniel Colascione, 65796 On Thu, 7 Sept 2023 at 09:07, Eli Zaretskii <eliz@gnu.org> wrote: > > > From: Xinyang Chen <chenxinyang99@gmail.com> > > Date: Wed, 6 Sep 2023 18:52:14 -0400 > > > > Currently `module_non_local_exit_get` returns pointers to fields > > in emacs_env_private: > > ``` > > if (p->pending_non_local_exit != emacs_funcall_exit_return) > > { > > *symbol = &p->non_local_exit_symbol; > > *data = &p->non_local_exit_data; > > } > > ``` > > this means that if one tries to: > > ``` > > funcall(...); > > non_local_exit_get(&s1, &d1); > > funcall(...); > > non_local_exit_get(&s2, &d2); > > non_local_exit_signal(s1, d1); > > ``` > > you would signal the second error, instead of the first error (I expected > > this to happen). > > It seems to me that `module_non_local_exit_get` should > > `allocate_emacs_value` instead. > > Philipp, Daniel: any comments? Nice find! We can't use allocate_emacs_value here because non_local_exit_get has to work in OOM situations and can never fail. What we could do here is e.g.: - Document the current behavior, stating that the emacs_value objects returned from non_local_exit_get are ephemeral. I'm not a huge fan of this because it makes non_local_exit_get behave different from all other functions. - Provide an alternative non_local_exit_copy that copies the 2 Lisp_Objects into an opaque buffer supplied by the user (plus a way to determine the buffer size). That way we shift the responsibility of dealing with allocation failures to the user. - Attempt to allocate a new emacs_value, fall back to the current behavior if that fails. I don't really like that option either because it doesn't solve the initial problem in all cases (so users still need to deal with it), but makes both the interface and the implementation more complex. - Crash if we can't allocate memory. That has been rejected in other cases. > > Btw, the non_local_exit_get function is currently not documented in > the ELisp manual; should it be? At least in Emacs 29 I see it documented ("Module Nonlocal" node). -- Google Germany GmbH Erika-Mann-Straße 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde. This e-mail is confidential. If you received this communication by mistake, please don’t forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person. ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#65796: dynamic module non_local_exit_get overwrites exit signals 2023-09-07 10:24 ` Philipp Stephani via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-07 10:55 ` Eli Zaretskii 0 siblings, 0 replies; 4+ messages in thread From: Eli Zaretskii @ 2023-09-07 10:55 UTC (permalink / raw) To: Philipp Stephani; +Cc: chenxinyang99, p.stephani2, dancol, 65796 > From: Philipp Stephani <phst@google.com> > Date: Thu, 7 Sep 2023 12:24:03 +0200 > Cc: Xinyang Chen <chenxinyang99@gmail.com>, Daniel Colascione <dancol@dancol.org>, 65796@debbugs.gnu.org, > p.stephani2@gmail.com > > Nice find! > We can't use allocate_emacs_value here because non_local_exit_get has > to work in OOM situations and can never fail. What we could do here is > e.g.: > - Document the current behavior, stating that the emacs_value objects > returned from non_local_exit_get are ephemeral. I'm not a huge fan of > this because it makes non_local_exit_get behave different from all > other functions. > - Provide an alternative non_local_exit_copy that copies the 2 > Lisp_Objects into an opaque buffer supplied by the user (plus a way to > determine the buffer size). That way we shift the responsibility of > dealing with allocation failures to the user. > - Attempt to allocate a new emacs_value, fall back to the current > behavior if that fails. I don't really like that option either because > it doesn't solve the initial problem in all cases (so users still need > to deal with it), but makes both the interface and the implementation > more complex. > - Crash if we can't allocate memory. That has been rejected in other cases. I guess just one alternative is acceptable? > > Btw, the non_local_exit_get function is currently not documented in > > the ELisp manual; should it be? > > At least in Emacs 29 I see it documented ("Module Nonlocal" node). I need new glasses. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-07 10:55 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-09-06 22:52 bug#65796: dynamic module non_local_exit_get overwrites exit signals Xinyang Chen 2023-09-07 7:07 ` Eli Zaretskii 2023-09-07 10:24 ` Philipp Stephani via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-09-07 10:55 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).