* Re: master 47b377f64b: Prevent non-local exits from ns-in-echo-area [not found] ` <20221113010439.BF21DC00AB5@vcs2.savannah.gnu.org> @ 2022-11-13 1:52 ` Stefan Monnier 2022-11-13 3:07 ` Po Lu 0 siblings, 1 reply; 3+ messages in thread From: Stefan Monnier @ 2022-11-13 1:52 UTC (permalink / raw) To: emacs-devel; +Cc: Po Lu Hmm... this still looks like an ugly hack, and still without any comments explaining why we're doing it. E.g. why do we need `safe_call` in addition to `internal_catch_all`? If we're using `safe_call`, why not use that code's handling of `inhibit_quit` and write our own around it instead? AFAICT we don't *really* know why we need `internal_catch_all` or `inhibit-quit` and even less if/why we need both. And we don't know what we *should* do if this code signals a weird error or `throw`s, really (currently, we just silently drop the error/throw, but maybe we should arrange for it to be "recreated" elsewhere). I'm not in a position to dig any further, and it looks like noone here is either, but *please* add a comment explaining as much as you can about what we know now, so as to help the future guy who wants to try and figure it out (or so as to avoid re-introducing the problem when someone stumbles upon this weird code full of redundant protections and figures it can be simplified). Stefan Po Lu via Mailing list for Emacs changes [2022-11-12 20:04:39] wrote: > branch: master > commit 47b377f64bef8c3da519b3aa9c5c90b7199ba524 > Author: Po Lu <luangruo@yahoo.com> > Commit: Po Lu <luangruo@yahoo.com> > > Prevent non-local exits from ns-in-echo-area > > * src/nsterm.m (ns_in_echo_area_1): > (ns_in_echo_area_2): > (ns_in_echo_area): New functions. > ([EmacsView firstRectForCharacterRange:]): Call them instead. > (syms_of_nsterm): New defsym. > --- > src/nsterm.m | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/src/nsterm.m b/src/nsterm.m > index 17f40dc7e3..507f2a9e7d 100644 > --- a/src/nsterm.m > +++ b/src/nsterm.m > @@ -7056,6 +7056,36 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) > processingCompose = NO; > } > > +static Lisp_Object > +ns_in_echo_area_1 (void *ptr) > +{ > + Lisp_Object in_echo_area; > + specpdl_ref count; > + > + count = SPECPDL_INDEX (); > + specbind (Qinhibit_quit, Qt); > + in_echo_area = safe_call (1, Qns_in_echo_area); > + > + return unbind_to (count, in_echo_area); > +} > + > +static Lisp_Object > +ns_in_echo_area_2 (enum nonlocal_exit exit, Lisp_Object error) > +{ > + return Qnil; > +} > + > +static bool > +ns_in_echo_area (void) > +{ > + Lisp_Object in_echo_area; > + > + in_echo_area > + = internal_catch_all (ns_in_echo_area_1, NULL, > + ns_in_echo_area_2); > + > + return !NILP (in_echo_area); > +} > > /* Used to position char selection windows, etc. */ > - (NSRect)firstRectForCharacterRange: (NSRange)theRange > @@ -7069,7 +7099,7 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) > if (NS_KEYLOG) > NSLog (@"firstRectForCharRange request"); > > - if (WINDOWP (echo_area_window) && ! NILP (call0 (intern ("ns-in-echo-area")))) > + if (WINDOWP (echo_area_window) && ns_in_echo_area ()) > win = XWINDOW (echo_area_window); > else > win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe)); > @@ -11012,6 +11042,7 @@ respectively. */); > DEFSYM (Qcondensed, "condensed"); > DEFSYM (Qreverse_italic, "reverse-italic"); > DEFSYM (Qexpanded, "expanded"); > + DEFSYM (Qns_in_echo_area, "ns-in-echo-area"); > > #ifdef NS_IMPL_COCOA > Fprovide (Qcocoa, Qnil); ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: master 47b377f64b: Prevent non-local exits from ns-in-echo-area 2022-11-13 1:52 ` master 47b377f64b: Prevent non-local exits from ns-in-echo-area Stefan Monnier @ 2022-11-13 3:07 ` Po Lu 2022-11-13 15:18 ` Stefan Monnier 0 siblings, 1 reply; 3+ messages in thread From: Po Lu @ 2022-11-13 3:07 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > Hmm... this still looks like an ugly hack, and still without any > comments explaining why we're doing it. > > E.g. why do we need `safe_call` in addition to `internal_catch_all`? > If we're using `safe_call`, why not use that code's handling of > `inhibit_quit` and write our own around it instead? safe_call binds inhibit-redisplay to t. > AFAICT we don't *really* know why we need `internal_catch_all` or > `inhibit-quit` and even less if/why we need both. We need both because throw-on-input can otherwise cause the code inside to throw, and as Gerd found out earlier it is not safe to call redisplay inside an NS input callback. Besides, it's Lisp, so a user could plausibly shoot himself in the foot by advising that function. I would rather write the entirety in C (see the calls to xic_set_preeditarea in xterm.c), but I have no idea how input methods work on NS. > And we don't know what we *should* do if this code signals a weird > error or `throw`s, really (currently, we just silently drop the > error/throw, but maybe we should arrange for it to be "recreated" > elsewhere). I don't know either, but it's better than having Lisp longjmp out of the NS code. > I'm not in a position to dig any further, and it looks like noone here > is either, but *please* add a comment explaining as much as you can > about what we know now, so as to help the future guy who wants to try > and figure it out (or so as to avoid re-introducing the problem when > someone stumbles upon this weird code full of redundant protections and > figures it can be simplified). Okay. How about: Use safe_call so redisplay is inhibited, and internal_catch_all so that the likes of `throw-on-input' or user advice can not cause a non-local exit out of the Lisp function being called, both of which are unsafe inside NS input callbacks. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: master 47b377f64b: Prevent non-local exits from ns-in-echo-area 2022-11-13 3:07 ` Po Lu @ 2022-11-13 15:18 ` Stefan Monnier 0 siblings, 0 replies; 3+ messages in thread From: Stefan Monnier @ 2022-11-13 15:18 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel > Okay. How about: > > Use safe_call so redisplay is inhibited, and internal_catch_all so > that the likes of `throw-on-input' or user advice can not cause a > non-local exit out of the Lisp function being called, both of which > are unsafe inside NS input callbacks. Fine by me, thanks, Stefan ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-13 15:18 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <166830147947.13137.4061439325094811374@vcs2.savannah.gnu.org> [not found] ` <20221113010439.BF21DC00AB5@vcs2.savannah.gnu.org> 2022-11-13 1:52 ` master 47b377f64b: Prevent non-local exits from ns-in-echo-area Stefan Monnier 2022-11-13 3:07 ` Po Lu 2022-11-13 15:18 ` Stefan Monnier
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.