From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Dynamic modules: MODULE_HANDLE_SIGNALS etc. Date: Sun, 3 Jan 2016 15:11:05 -0800 Organization: UCLA Computer Science Department Message-ID: <5689AA89.4030404@cs.ucla.edu> References: <83mvu1x6t3.fsf@gnu.org> <56772054.8010401@cs.ucla.edu> <83zix4scgf.fsf@gnu.org> <5677DBC9.6030307@cs.ucla.edu> <83io3rst2r.fsf@gnu.org> <567841A6.4090408@cs.ucla.edu> <567844B9.2050308@dancol.org> <5678CD07.8080209@cs.ucla.edu> <5678D3AF.7030101@dancol.org> <5678D620.6070000@cs.ucla.edu> <83mvt2qxm1.fsf@gnu.org> <56797CD9.8010706@cs.ucla.edu> <8337uuqsux.fsf@gnu.org> <5679DC83.70405@cs.ucla.edu> <83oadhp2mj.fsf@gnu.org> <567AD556.6020202@cs.ucla.edu> <567AD766.3060608@dancol.org> <567B5DAB.2000900@cs.ucla.edu> <83fuyromig.fsf@gnu.org> <567C25B1.3020101@dancol.org> <56892FD6.8040708@dancol.org> <56894CE7.7090301@cs.ucla.edu> <568950C5.2030306@dancol.org> <56896359.4020309@cs.ucla.edu> <568966D4.5080707@dancol.org> <56898C6F.4010303@cs.ucla.edu> <56898EBD.2000000@dancol.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------070308030909010406030506" X-Trace: ger.gmane.org 1451862698 16607 80.91.229.3 (3 Jan 2016 23:11:38 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 3 Jan 2016 23:11:38 +0000 (UTC) To: Daniel Colascione , Eli Zaretskii , Emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Jan 04 00:11:21 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aFro0-0007kB-VL for ged-emacs-devel@m.gmane.org; Mon, 04 Jan 2016 00:11:21 +0100 Original-Received: from localhost ([::1]:43177 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aFro0-0004dN-2l for ged-emacs-devel@m.gmane.org; Sun, 03 Jan 2016 18:11:20 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aFrnu-0004dG-B4 for Emacs-devel@gnu.org; Sun, 03 Jan 2016 18:11:16 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aFrnp-0003zh-58 for Emacs-devel@gnu.org; Sun, 03 Jan 2016 18:11:14 -0500 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:47918) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aFrno-0003yo-Rc; Sun, 03 Jan 2016 18:11:09 -0500 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 11222160D97; Sun, 3 Jan 2016 15:11:08 -0800 (PST) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id aNO-wWsArnJm; Sun, 3 Jan 2016 15:11:06 -0800 (PST) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 4001D160817; Sun, 3 Jan 2016 15:11:06 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id pdj-DwQoArh6; Sun, 3 Jan 2016 15:11:06 -0800 (PST) Original-Received: from [192.168.1.9] (pool-100-32-155-148.lsanca.fios.verizon.net [100.32.155.148]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 114F4160095; Sun, 3 Jan 2016 15:11:06 -0800 (PST) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 In-Reply-To: <56898EBD.2000000@dancol.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:197520 Archived-At: This is a multi-part message in MIME format. --------------070308030909010406030506 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Daniel Colascione wrote: > It's *critical* not to violate the invariants of code. Sure, but we are discussing what those invariants should be; they are not carved in stone. One possible invariant is (A) "stack overflow never happens". Another is (B) "if stack overflow happens, callers must tolerate being longjmped through". Either invariant is reasonable per se. It is a judgment call as to which invariant is better for Emacs. Possibly some modules will prefer (A) and others (B). Take the regular expression code as an example. Suppose it has unusual worst-case behavior that can grow the stack in arbitrary ways (which I think it does though I'm not going to investigate the details right now). One way to address this is to rewrite the code so that it doesn't have the behavior, but that would be a pain; the code has been that way for decades and is crufty at this point and a lot of Emacs depends on its quirks. Another way to address it is to use a guard page or whatever on the halfway-decent platforms that support that sort of thing. We've chosen the latter, i.e., we've chosen invariant (B), and yes there are problems with this approach but it beats doing nothing and it beats doing (A) because nobody has had the time to do (A), assuming it's doable at all. > It should be possible to replace the printfs > in this instance with calls to write(1, "message") (which will bypass > any output buffering) and restore async-signal-safety. Good point. I did that with the attached patch to emacs-25. However, this doesn't address the Fdo_auto_save () issue in the same neighborhood. > If a user elects to attempt auto-save in this situation, that's on him. Sure, and Emacs already asks the user whether to auto-save in that situation, so this should be OK already. > Ideally, we'd make autosave async-signal-safe, which will help in this > handler and in the segfault hander. Yes, that'd be good, if we didn't lose functionality thereby. --------------070308030909010406030506 Content-Type: text/x-diff; name="0001-Avoid-stdio-in-SIGINT-handler.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-Avoid-stdio-in-SIGINT-handler.patch" =46rom d8a33374cfa7deaf1e2fd4762c59bc6607850f65 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 3 Jan 2016 15:00:49 -0800 Subject: [PATCH] Avoid stdio in SIGINT handler * admin/merge-gnulib (GNULIB_MODULES): Add ignore-value. * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. * lib/ignore-value.h: New file, from gnulib. * src/keyboard.c: Include it. (write_stdout, read_stdin): New functions. (handle_interrupt): Use them instead of printf and getchar, and avoid fflush when handling signals. --- admin/merge-gnulib | 2 +- lib/gnulib.mk | 9 ++++++- lib/ignore-value.h | 50 +++++++++++++++++++++++++++++++++++++ m4/gnulib-comp.m4 | 2 ++ src/keyboard.c | 73 +++++++++++++++++++++++++++++++++++-------------= ------ 5 files changed, 108 insertions(+), 28 deletions(-) create mode 100644 lib/ignore-value.h diff --git a/admin/merge-gnulib b/admin/merge-gnulib index 363bb23..40b5b78 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -32,7 +32,7 @@ GNULIB_MODULES=3D' dtoastr dtotimespec dup2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog - intprops largefile lstat + ignore-value intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time stdalign stddef stdio diff --git a/lib/gnulib.mk b/lib/gnulib.mk index 97ed5b1..b920cbb 100644 --- a/lib/gnulib.mk +++ b/lib/gnulib.mk @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --lib=3Dlibgnu --source-base=3Dlib = --m4-base=3Dm4 --doc-base=3Ddoc --tests-base=3Dtests --aux-dir=3Dbuild-au= x --avoid=3Dclose --avoid=3Ddup --avoid=3Dfchdir --avoid=3Dflexmember --a= void=3Dfstat --avoid=3Dmalloc-posix --avoid=3Dmsvc-inval --avoid=3Dmsvc-n= othrow --avoid=3Dopen --avoid=3Dopenat-die --avoid=3Dopendir --avoid=3Dra= ise --avoid=3Dsave-cwd --avoid=3Dselect --avoid=3Dsetenv --avoid=3Dsigpro= cmask --avoid=3Dstdarg --avoid=3Dstdbool --avoid=3Dthreadlib --avoid=3Dun= setenv --makefile-name=3Dgnulib.mk --conditional-dependencies --no-libtoo= l --macro-prefix=3Dgl --no-vc-files alloca-opt binary-io byteswap c-ctype= c-strcase careadlinkat close-stream count-one-bits count-trailing-zeros = crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec du= p2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode = fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-change= log intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 p= select pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str sockl= en stat-time stdalign stddef stdio stpcpy strftime strtoimax strtoumax sy= mlink sys_stat sys_time time time_r time_rz timegm timer-time timespec-ad= d timespec-sub unsetenv update-copyright utimens vla warnings +# Reproduce by: gnulib-tool --import --lib=3Dlibgnu --source-base=3Dlib = --m4-base=3Dm4 --doc-base=3Ddoc --tests-base=3Dtests --aux-dir=3Dbuild-au= x --avoid=3Dclose --avoid=3Ddup --avoid=3Dfchdir --avoid=3Dflexmember --a= void=3Dfstat --avoid=3Dmalloc-posix --avoid=3Dmsvc-inval --avoid=3Dmsvc-n= othrow --avoid=3Dopen --avoid=3Dopenat-die --avoid=3Dopendir --avoid=3Dra= ise --avoid=3Dsave-cwd --avoid=3Dselect --avoid=3Dsetenv --avoid=3Dsigpro= cmask --avoid=3Dstdarg --avoid=3Dstdbool --avoid=3Dthreadlib --avoid=3Dun= setenv --makefile-name=3Dgnulib.mk --conditional-dependencies --no-libtoo= l --macro-prefix=3Dgl --no-vc-files alloca-opt binary-io byteswap c-ctype= c-strcase careadlinkat close-stream count-one-bits count-trailing-zeros = crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec du= p2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode = fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-change= log ignore-value intprops largefile lstat manywarnings memrchr mkostemp m= ktime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat = sig2str socklen stat-time stdalign stddef stdio stpcpy strftime strtoimax= strtoumax symlink sys_stat sys_time time time_r time_rz timegm timer-tim= e timespec-add timespec-sub unsetenv update-copyright utimens vla warning= s =20 =20 MOSTLYCLEANFILES +=3D core *.stackdump @@ -567,6 +567,13 @@ EXTRA_libgnu_a_SOURCES +=3D group-member.c =20 ## end gnulib module group-member =20 +## begin gnulib module ignore-value + + +EXTRA_DIST +=3D ignore-value.h + +## end gnulib module ignore-value + ## begin gnulib module intprops =20 =20 diff --git a/lib/ignore-value.h b/lib/ignore-value.h new file mode 100644 index 0000000..6713d96 --- /dev/null +++ b/lib/ignore-value.h @@ -0,0 +1,50 @@ +/* ignore a function return without a compiler warning. -*- coding: utf= -8 -*- + + Copyright (C) 2008-2016 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see .= */ + +/* Written by Jim Meyering, Eric Blake and P=C3=A1draig Brady. */ + +/* Use "ignore_value" to avoid a warning when using a function declared = with + gcc's warn_unused_result attribute, but for which you really do want = to + ignore the result. Traditionally, people have used a "(void)" cast t= o + indicate that a function's return value is deliberately unused. Howe= ver, + if the function is declared with __attribute__((warn_unused_result)),= + gcc issues a warning even with the cast. + + Caution: most of the time, you really should heed gcc's warning, and + check the return value. However, in those exceptional cases in which= + you're sure you know what you're doing, use this function. + + For the record, here's one of the ignorable warnings: + "copy.c:233: warning: ignoring return value of 'fchown', + declared with attribute warn_unused_result". */ + +#ifndef _GL_IGNORE_VALUE_H +#define _GL_IGNORE_VALUE_H + +/* Normally casting an expression to void discards its value, but GCC + versions 3.4 and newer have __attribute__ ((__warn_unused_result__)) + which may cause unwanted diagnostics in that case. Use __typeof__ + and __extension__ to work around the problem, if the workaround is + known to be needed. */ +#if 3 < __GNUC__ + (4 <=3D __GNUC_MINOR__) +# define ignore_value(x) \ + (__extension__ ({ __typeof__ (x) __x =3D (x); (void) __x; })) +#else +# define ignore_value(x) ((void) (x)) +#endif + +#endif diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index 69920a8..27ca70a 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -91,6 +91,7 @@ AC_DEFUN([gl_EARLY], # Code from module gettimeofday: # Code from module gitlog-to-changelog: # Code from module group-member: + # Code from module ignore-value: # Code from module include_next: # Code from module intprops: # Code from module inttypes-incomplete: @@ -905,6 +906,7 @@ AC_DEFUN([gl_FILE_LIST], [ lib/gettimeofday.c lib/gl_openssl.h lib/group-member.c + lib/ignore-value.h lib/intprops.h lib/inttypes.in.h lib/lstat.c diff --git a/src/keyboard.c b/src/keyboard.c index fcafd0b..6bdfc1a 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -64,6 +64,8 @@ along with GNU Emacs. If not, see . */ #include #include =20 +#include + #ifdef HAVE_WINDOW_SYSTEM #include TERM_HEADER #endif /* HAVE_WINDOW_SYSTEM */ @@ -10206,6 +10208,21 @@ deliver_interrupt_signal (int sig) deliver_process_signal (sig, handle_interrupt_signal); } =20 +/* Output MSG directly to standard output, without buffering. Ignore + failures. This is safe in a signal handler. */ +static void +write_stdout (char const *msg) +{ + ignore_value (write (STDOUT_FILENO, msg, strlen (msg))); +} + +/* Read a byte from stdin, without buffering. Safe in signal handlers. = */ +static int +read_stdin (void) +{ + char c; + return read (STDIN_FILENO, &c, 1) =3D=3D 1 ? c : EOF; +} =20 /* If Emacs is stuck because `inhibit-quit' is true, then keep track of the number of times C-g has been requested. If C-g is pressed @@ -10242,9 +10259,9 @@ handle_interrupt (bool in_signal_handler) sigemptyset (&blocked); sigaddset (&blocked, SIGINT); pthread_sigmask (SIG_BLOCK, &blocked, 0); + fflush (stdout); } =20 - fflush (stdout); reset_all_sys_modes (); =20 #ifdef SIGTSTP @@ -10260,8 +10277,9 @@ handle_interrupt (bool in_signal_handler) /* Perhaps should really fork an inferior shell? But that would not provide any way to get back to the original shell, ever. */ - printf ("No support for stopping a process on this operating syste= m;\n"); - printf ("you can continue or abort.\n"); + write_stdout ("No support for stopping a process" + " on this operating system;\n" + "you can continue or abort.\n"); #endif /* not SIGTSTP */ #ifdef MSDOS /* We must remain inside the screen area when the internal termina= l @@ -10272,46 +10290,49 @@ handle_interrupt (bool in_signal_handler) the code used for auto-saving doesn't cope with the mark bit. */ if (!gc_in_progress) { - printf ("Auto-save? (y or n) "); - fflush (stdout); - if (((c =3D getchar ()) & ~040) =3D=3D 'Y') + write_stdout ("Auto-save? (y or n) "); + c =3D read_stdin (); + if ((c & 040) =3D=3D 'Y') { Fdo_auto_save (Qt, Qnil); #ifdef MSDOS - printf ("\r\nAuto-save done"); -#else /* not MSDOS */ - printf ("Auto-save done\n"); -#endif /* not MSDOS */ + write_stdout ("\r\nAuto-save done"); +#else + write_stdout ("Auto-save done\n"); +#endif } - while (c !=3D '\n') c =3D getchar (); + while (c !=3D '\n') + c =3D read_stdin (); } else { /* During GC, it must be safe to reenable quitting again. */ Vinhibit_quit =3D Qnil; + write_stdout + ( #ifdef MSDOS - printf ("\r\n"); -#endif /* not MSDOS */ - printf ("Garbage collection in progress; cannot auto-save now\r\n"); - printf ("but will instead do a real quit after garbage collection end= s\r\n"); - fflush (stdout); + "\r\n" +#endif + "Garbage collection in progress; cannot auto-save now\r\n" + "but will instead do a real quit" + " after garbage collection ends\r\n"); } =20 #ifdef MSDOS - printf ("\r\nAbort? (y or n) "); -#else /* not MSDOS */ - printf ("Abort (and dump core)? (y or n) "); -#endif /* not MSDOS */ - fflush (stdout); - if (((c =3D getchar ()) & ~040) =3D=3D 'Y') + write_stdout ("\r\nAbort? (y or n) "); +#else + write_stdout ("Abort (and dump core)? (y or n) "); +#endif + c =3D read_stdin (); + if ((c & ~040) =3D=3D 'Y') emacs_abort (); - while (c !=3D '\n') c =3D getchar (); + while (c !=3D '\n') + c =3D read_stdin (); #ifdef MSDOS - printf ("\r\nContinuing...\r\n"); + write_stdout ("\r\nContinuing...\r\n"); #else /* not MSDOS */ - printf ("Continuing...\n"); + write_stdout ("Continuing...\n"); #endif /* not MSDOS */ - fflush (stdout); init_all_sys_modes (); } else --=20 2.5.0 --------------070308030909010406030506--