On 08/31/2012 07:29 AM, Eli Zaretskii wrote: > why not simply replace all the calls > to 'abort' with calls to 'emacs_abort'? Isn't that cleaner? Yes, I prefer that, too. I avoided that only because it bloated the patch. But it's easy enough to add, and the attached patch does that. >> To keep in sync with that, the MS-Windows part of the patch >> renames w32_abort to emacs_abort. > > I don't see this part in your patches. Where is it? Sorry, I missed one place. Fixed in attached patch. This also should fix the HAVE_NTGUI issue you reported in a separate email. > with your changes, only those calls to 'abort' that > originate within Emacs sources will wind up at 'emacs_abort'. Yes, that's part of the idea. Intercepting other libraries' call to 'abort' tends to break things. We've caught some of the breakages and have disabled them with NO_ABORT but it's safer to avoid the problem in the first place. Doing this will also make it a bit easier for me to add a new feature, namely backtraces on production aborts. > A standard header should not include another > standard header That guideline is needed for glibc for conformance reasons, and it used to be important long ago when multiple inclusion of standard headers was a problem, but these days it's not necessary and is even somewhat counterproductive in portability emulations. We violate that guideline all the time in gnulib (and therefore in Emacs) and it's not a problem. However, if you prefer using the glibc style for the Microsoft port that's easy to do I've revised the patch to do that. The attached patch is fairly long because of the abort -> emacs_abort thing, so here's a precis that skips the boring parts. === modified file 'ChangeLog' --- ChangeLog 2012-08-28 16:01:59 +0000 +++ ChangeLog 2012-08-31 03:18:42 +0000 @@ -1,3 +1,8 @@ +2012-08-31 Paul Eggert + + Simplify redefinition of 'abort' (Bug#12316). + * configure.ac (NO_ABRT): Remove. + 2012-08-26 Paul Eggert * configure.ac (CFLAGS): Prefer -g3 to -g if -g3 works === modified file 'configure.ac' --- configure.ac 2012-08-28 16:01:59 +0000 +++ configure.ac 2012-08-30 18:23:15 +0000 @@ -3321,12 +3321,6 @@ AC_DEFINE(BROKEN_PTY_READ_AFTER_EAGAIN, 1, [Define on FreeBSD to work around an issue when reading from a PTY.]) ;; - - dnl Define the following so emacs symbols will not conflict with those - dnl in the System framework. Otherwise -prebind will not work. - darwin) - AC_DEFINE(NO_ABORT, 1, [Do not define abort in emacs.c.]) - ;; esac case $opsys in === modified file 'nt/ChangeLog' --- nt/ChangeLog 2012-08-28 16:01:59 +0000 +++ nt/ChangeLog 2012-08-31 16:26:56 +0000 @@ -1,3 +1,8 @@ +2012-08-31 Paul Eggert + + Simplify redefinition of 'abort' (Bug#12316). + * inc/ms-w32.h (w32_abort) [HAVE_NTGUI]: Remove. + 2012-08-22 Juanma Barranquero * config.nt: Sync with autogen/config.in. === modified file 'nt/inc/ms-w32.h' --- nt/inc/ms-w32.h 2012-08-07 11:03:48 +0000 +++ nt/inc/ms-w32.h 2012-08-30 22:25:36 +0000 @@ -334,16 +334,7 @@ #include #endif -/* stdlib.h must be included after redefining malloc & friends, but - before redefining abort. Isn't library redefinition funny? */ #include - -/* Redefine abort. */ -#ifdef HAVE_NTGUI -#define abort w32_abort -extern _Noreturn void w32_abort (void); -#endif - #include /* Define for those source files that do not include enough NT system files. */ === modified file 'nt/inc/unistd.h' --- nt/inc/unistd.h 2011-02-27 19:48:31 +0000 +++ nt/inc/unistd.h 2012-08-31 16:31:23 +0000 @@ -3,8 +3,12 @@ #ifndef _UNISTD_H #define _UNISTD_H +/* On Microsoft platforms, declares 'environ'; on POSIX + platforms, does. Every file in Emacs that includes + also includes , so there's no need to declare + 'environ' here. */ + extern ssize_t readlink (const char *, char *, size_t); extern int symlink (char const *, char const *); #endif /* _UNISTD_H */ - === modified file 'src/.gdbinit' --- src/.gdbinit 2012-08-20 17:32:31 +0000 +++ src/.gdbinit 2012-08-30 19:23:45 +0000 @@ -1222,14 +1222,9 @@ set $tem = (struct Lisp_String *) $ptr set $tem = (char *) $tem->data - # Don't let abort actually run, as it will make stdio stop working and - # therefore the `pr' command above as well. - if $tem[0] == 'w' && $tem[1] == 'i' && $tem[2] == 'n' && $tem[3] == 'd' - # The windows-nt build replaces abort with its own function. - break w32_abort - else - break abort - end + # Don't let emacs_abort actually run, as it will make stdio stop + # working and therefore the 'pr' command above as well. + break emacs_abort end # x_error_quitter is defined only on X. But window-system is set up === modified file 'src/ChangeLog' --- src/ChangeLog 2012-08-31 10:53:19 +0000 +++ src/ChangeLog 2012-08-31 16:34:18 +0000 @@ -1,3 +1,23 @@ +2012-08-31 Paul Eggert + + Simplify redefinition of 'abort' (Bug#12316). + Do not try to redefine the 'abort' function. Instead, redo + the code so that it calls 'emacs_abort' rather than 'abort'. + This removes the need for the NO_ABORT configure-time macro + and makes it easier to change the abort code to do a backtrace. + * .gdbinit: Just stop at emacs_abort, not at w32_abort or abort. + * emacs.c (abort) [!DOS_NT && !NO_ABORT]: + Remove; sysdep.c's emacs_abort now takes its place. + * lisp.h (emacs_abort): New decl. All calls from Emacs code to + 'abort' changed to use 'emacs_abort'. + * msdos.c (dos_abort) [defined abort]: Remove; not used. + (abort) [!defined abort]: Rename to ... + (emacs_abort): ... new name. + * sysdep.c (emacs_abort) [!HAVE_NTGUI]: New function, taking + the place of the old 'abort' in emacs.c. + * w32.c, w32fns.c (abort): Do not #undef. + * w32.c (emacs_abort): Rename from w32_abort. + 2012-08-31 Dmitry Antipov Remove mark_ttys function and fix tty_display_info initialization. === modified file 'src/conf_post.h' --- src/conf_post.h 2012-08-20 16:48:10 +0000 +++ src/conf_post.h 2012-08-31 14:47:20 +0000 @@ -178,9 +178,6 @@ #endif #include -/* If you think about removing the line below, note that the - MS-Windows build relies on it for declaration of 'environ' needed - by a few source files. */ #include #if __GNUC__ >= 3 /* On GCC 3.0 we might get a warning. */ === modified file 'src/emacs.c' --- src/emacs.c 2012-08-25 06:38:43 +0000 +++ src/emacs.c 2012-08-31 14:58:40 +0000 @@ -345,22 +345,6 @@ } #endif -/* We define abort, rather than using it from the library, - so that GDB can return from a breakpoint here. - MSDOS has its own definition in msdos.c. */ - -#if ! defined (DOS_NT) && ! defined (NO_ABORT) - -void -abort (void) -{ - kill (getpid (), SIGABRT); - /* This shouldn't be executed, but it prevents a warning. */ - exit (1); -} -#endif - - /* Code for dealing with Lisp access to the Unix command line. */ static void === modified file 'src/msdos.c' --- src/msdos.c 2012-08-21 10:21:04 +0000 +++ src/msdos.c 2012-08-31 14:56:42 +0000 @@ -4215,26 +4215,8 @@ } #endif -#ifdef abort -#undef abort void -dos_abort (char *file, int line) -{ - char buffer1[200], buffer2[400]; - int i, j; - - sprintf (buffer1, "", file, line); - for (i = j = 0; buffer1[i]; i++) { - buffer2[j++] = buffer1[i]; - buffer2[j++] = 0x70; - } - dosmemput (buffer2, j, (int)ScreenPrimary); - ScreenSetCursor (2, 0); - abort (); -} -#else -void -abort (void) +emacs_abort (void) { dos_ttcooked (); ScreenSetCursor (10, 0); @@ -4250,7 +4232,6 @@ #endif /* __DJGPP_MINOR__ >= 2 */ exit (2); } -#endif void syms_of_msdos (void) === modified file 'src/sysdep.c' --- src/sysdep.c 2012-08-18 02:49:24 +0000 +++ src/sysdep.c 2012-08-30 22:25:36 +0000 @@ -1838,6 +1838,15 @@ } #endif +#ifndef HAVE_NTGUI +/* Using emacs_abort lets GDB return from a breakpoint here. */ +void +emacs_abort (void) +{ + (abort) (); +} +#endif + int emacs_open (const char *path, int oflag, int mode) { === modified file 'src/w32.c' --- src/w32.c 2012-08-26 10:29:37 +0000 +++ src/w32.c 2012-08-31 15:03:25 +0000 @@ -6640,8 +6640,7 @@ buffer, "Emacs Abort Dialog", MB_OK | MB_ICONEXCLAMATION | MB_TASKMODAL); - /* Use the low-level Emacs abort. */ -#undef abort + /* Use the low-level system abort. */ abort (); } else === modified file 'src/w32fns.c' --- src/w32fns.c 2012-08-18 06:06:39 +0000 +++ src/w32fns.c 2012-08-31 15:04:03 +0000 @@ -7194,10 +7194,8 @@ syms_of_w32uniscribe (); } -#undef abort - void -w32_abort (void) +emacs_abort (void) { int button; button = MessageBox (NULL,