From f99c65e5743526a7fcc6352599b6f0efd3970202 Mon Sep 17 00:00:00 2001 From: Jan Djärv Date: Tue, 13 Nov 2012 08:56:15 +0100 Subject: * nsterm.m (hold_event): Send SIGIO to make sure ns_read_socket is called. Fixes: debbugs:12834 --- src/ChangeLog | 5 +++++ src/nsterm.m | 2 ++ 2 files changed, 7 insertions(+) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 494b2179516..d72091c0ed6 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2012-11-13 Jan Djärv + + * nsterm.m (hold_event): Send SIGIO to make sure ns_read_socket is + called (Bug#12834). + 2012-11-12 Eli Zaretskii * xdisp.c (decode_mode_spec): Limit the value of WIDTH argument diff --git a/src/nsterm.m b/src/nsterm.m index 9b2e544c75b..f4982e0a7cb 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -312,6 +312,8 @@ hold_event (struct input_event *event) } hold_event_q.q[hold_event_q.nr++] = *event; + /* Make sure ns_read_socket is called, i.e. we have input. */ + kill (0, SIGIO); } static Lisp_Object -- cgit v1.2.1 From 73dcdb9f30cb94a3183db54d9b463370c3978d4d Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 13 Nov 2012 20:55:41 -0800 Subject: Use faccessat, not access, when checking file permissions. This fixes a bug that has been present in Emacs since its creation. It was reported by Chris Torek in 1983 even before GNU Emacs existed, which must set some sort of record. (Torek's bug report was against a predecessor of GNU Emacs, but GNU Emacs happened to have the same common flaw.) See Torek's Usenet posting "setuid/setgid programs & Emacs" Article-I.D.: sri-arpa.858 Posted: Fri Apr 8 14:18:56 1983. * .bzrignore: Add lib/fcntl.h. * configure.ac (euidaccess): Remove check; gnulib does this for us now. (gl_FCNTL_O_FLAGS): Define a dummy version. * lib/at-func.c, lib/euidaccess.c, lib/faccessat.c, lib/fcntl.in.h: * lib/getgroups.c, lib/group-member.c, lib/root-uid.h: * lib/xalloc-oversized.h, m4/euidaccess.m4, m4/faccessat.m4: * m4/fcntl_h.m4, m4/getgroups.m4, m4/group-member.m4: New files, from gnulib. * lib/gnulib.mk, m4/gnulib-comp.m4: Regenerate. * admin/merge-gnulib (GNULIB_MODULES): Add faccessat. (GNULIB_TOOL_FLAGS): Avoid at-internal, fchdir, malloc-posix, openat-die, openat-h, save-cwd. Do not avoid fcntl-h. Omit gnulib's m4/fcntl-o.m4. * nt/inc/ms-w32.h (AT_FDCWD, AT_EACCESS): New symbols. (access): Remove. (faccessat): New macro. * src/Makefile.in (LIB_EACCESS): New macro. (LIBES): Use it. * src/callproc.c (init_callproc): * src/charset.c (init_charset): * src/fileio.c (check_existing, check_executable, check_writable) (Ffile_readable_p): * src/lread.c (openp, load_path_check): * src/process.c (allocate_pty): * src/xrdb.c (file_p): Use effective UID when checking permissions, not real UID. * src/callproc.c (init_callproc): * src/charset.c (init_charset): * src/lread.c (load_path_check, init_lread): Test whether directories are accessible, not merely whether they exist. * src/conf_post.h (GNULIB_SUPPORT_ONLY_AT_FDCWD): New macro. * src/fileio.c (check_existing, check_executable, check_writable) (Ffile_readable_p): Use symbolic names instead of integers for the flags, as they're portable now. (check_writable): New arg AMODE. All uses changed. Set errno on failure. (Ffile_readable_p): Use faccessat, not stat + open + close. (Ffile_writable_p): No need to call check_existing + check_writable. Just call check_writable and then look at errno. This saves a syscall. dir should never be nil; replace an unnecessary runtime check with an eassert. When checking the parent directory of a nonexistent file, check that the directory is searchable as well as writable, as we can't create files in unsearchable directories. (file_directory_p): New function, which uses 'stat' on most platforms but faccessat with D_OK (for efficiency) if WINDOWSNT. (Ffile_directory_p, Fset_file_times): Use it. (file_accessible_directory_p): New function, which uses a single syscall for efficiency. (Ffile_accessible_directory_p): Use it. * src/xrdb.c (file_p): Use file_directory_p. * src/lisp.h (file_directory_p, file_accessible_directory_p): New decls. * src/lread.c (openp): When opening a file, use fstat rather than stat, as that avoids a permissions race. When not opening a file, use file_directory_p rather than stat. (dir_warning): First arg is now a usage string, not a format. Use errno. All uses changed. * src/nsterm.m (ns_term_init): Remove unnecessary call to file-readable that merely introduced a race. * src/process.c, src/sysdep.c, src/term.c: All uses of '#ifdef O_NONBLOCK' changed to '#if O_NONBLOCK', to accommodate gnulib O_* style, and similarly for the other O_* flags. * src/w32.c (sys_faccessat): Rename from sys_access and switch to faccessat's API. All uses changed. * src/xrdb.c: Do not include ; no longer needed. (magic_db): Rename from magic_file_p. (magic_db, search_magic_path): Return an XrmDatabase rather than a char *, so that we don't have to test for file existence separately from opening the file for reading. This removes a race fixes a permission-checking problem, and simplifies the code. All uses changed. (file_p): Remove; no longer needed. Fixes: debbugs:12632 --- src/ChangeLog | 67 +++++++++++++++++++++ src/Makefile.in | 3 +- src/callproc.c | 12 ++-- src/charset.c | 2 +- src/conf_post.h | 4 ++ src/fileio.c | 184 ++++++++++++++++++++++++++++---------------------------- src/lisp.h | 2 + src/lread.c | 92 +++++++++++++++------------- src/nsterm.m | 2 - src/process.c | 36 +++++------ src/sysdep.c | 6 +- src/term.c | 4 +- src/w32.c | 18 ++++-- src/xrdb.c | 101 ++++++++++++------------------- 14 files changed, 297 insertions(+), 236 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 1d94cef6577..a6b42e8a58c 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,70 @@ +2012-11-14 Paul Eggert + + Use faccessat, not access, when checking file permissions (Bug#12632). + This fixes a bug that has been present in Emacs since its creation. + It was reported by Chris Torek in 1983 even before GNU Emacs existed, + which must set some sort of record. (Torek's bug report was against + a predecessor of GNU Emacs, but GNU Emacs happened to have the + same common flaw.) See Torek's Usenet posting + "setuid/setgid programs & Emacs" Article-I.D.: sri-arpa.858 + Posted: Fri Apr 8 14:18:56 1983. + * Makefile.in (LIB_EACCESS): New macro. + (LIBES): Use it. + * callproc.c (init_callproc): + * charset.c (init_charset): + * fileio.c (check_existing, check_executable, check_writable) + (Ffile_readable_p): + * lread.c (openp, load_path_check): + * process.c (allocate_pty): + * xrdb.c (file_p): + Use effective UID when checking permissions, not real UID. + * callproc.c (init_callproc): + * charset.c (init_charset): + * lread.c (load_path_check, init_lread): + Test whether directories are accessible, not merely whether they exist. + * conf_post.h (GNULIB_SUPPORT_ONLY_AT_FDCWD): New macro. + * fileio.c (check_existing, check_executable, check_writable) + (Ffile_readable_p): + Use symbolic names instead of integers for the flags, as they're + portable now. + (check_writable): New arg AMODE. All uses changed. + Set errno on failure. + (Ffile_readable_p): Use faccessat, not stat + open + close. + (Ffile_writable_p): No need to call check_existing + check_writable. + Just call check_writable and then look at errno. This saves a syscall. + dir should never be nil; replace an unnecessary runtime check + with an eassert. When checking the parent directory of a nonexistent + file, check that the directory is searchable as well as writable, as + we can't create files in unsearchable directories. + (file_directory_p): New function, which uses 'stat' on most platforms + but faccessat with D_OK (for efficiency) if WINDOWSNT. + (Ffile_directory_p, Fset_file_times): Use it. + (file_accessible_directory_p): New function, which uses a single + syscall for efficiency. + (Ffile_accessible_directory_p): Use it. + * xrdb.c (file_p): Use file_directory_p. + * lisp.h (file_directory_p, file_accessible_directory_p): New decls. + * lread.c (openp): When opening a file, use fstat rather than + stat, as that avoids a permissions race. When not opening a file, + use file_directory_p rather than stat. + (dir_warning): First arg is now a usage string, not a format. + Use errno. All uses changed. + * nsterm.m (ns_term_init): Remove unnecessary call to file-readable + that merely introduced a race. + * process.c, sysdep.c, term.c: All uses of '#ifdef O_NONBLOCK' + changed to '#if O_NONBLOCK', to accommodate gnulib O_* style, + and similarly for the other O_* flags. + * w32.c (sys_faccessat): Rename from sys_access and switch to + faccessat's API. All uses changed. + * xrdb.c: Do not include ; no longer needed. + (magic_db): Rename from magic_file_p. + (magic_db, search_magic_path): Return an XrmDatabase rather than a + char *, so that we don't have to test for file existence + separately from opening the file for reading. This removes a race + fixes a permission-checking problem, and simplifies the code. + All uses changed. + (file_p): Remove; no longer needed. + 2012-11-13 Dmitry Antipov Omit glyphs initialization at startup. diff --git a/src/Makefile.in b/src/Makefile.in index c24e421bbbc..d034ad04796 100644 --- a/src/Makefile.in +++ b/src/Makefile.in @@ -150,6 +150,7 @@ M17N_FLT_CFLAGS = @M17N_FLT_CFLAGS@ M17N_FLT_LIBS = @M17N_FLT_LIBS@ LIB_CLOCK_GETTIME=@LIB_CLOCK_GETTIME@ +LIB_EACCESS=@LIB_EACCESS@ LIB_TIMER_TIME=@LIB_TIMER_TIME@ DBUS_CFLAGS = @DBUS_CFLAGS@ @@ -392,7 +393,7 @@ otherobj= $(TERMCAP_OBJ) $(PRE_ALLOC_OBJ) $(GMALLOC_OBJ) $(RALLOC_OBJ) \ LIBES = $(LIBS) $(W32_LIBS) $(LIBX_BASE) $(LIBIMAGE) \ $(LIBX_OTHER) $(LIBSOUND) \ $(RSVG_LIBS) $(IMAGEMAGICK_LIBS) $(LIB_CLOCK_GETTIME) \ - $(LIB_TIMER_TIME) $(DBUS_LIBS) \ + $(LIB_EACCESS) $(LIB_TIMER_TIME) $(DBUS_LIBS) \ $(LIB_EXECINFO) \ $(LIBXML2_LIBS) $(LIBGPM) $(LIBRESOLV) $(LIBS_SYSTEM) \ $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \ diff --git a/src/callproc.c b/src/callproc.c index c7bbe36e605..8ecaba2b408 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -1576,15 +1576,13 @@ init_callproc (void) #endif { tempdir = Fdirectory_file_name (Vexec_directory); - if (access (SSDATA (tempdir), 0) < 0) - dir_warning ("Warning: arch-dependent data dir (%s) does not exist.\n", - Vexec_directory); + if (! file_accessible_directory_p (SSDATA (tempdir))) + dir_warning ("arch-dependent data dir", Vexec_directory); } tempdir = Fdirectory_file_name (Vdata_directory); - if (access (SSDATA (tempdir), 0) < 0) - dir_warning ("Warning: arch-independent data dir (%s) does not exist.\n", - Vdata_directory); + if (! file_accessible_directory_p (SSDATA (tempdir))) + dir_warning ("arch-independent data dir", Vdata_directory); sh = (char *) getenv ("SHELL"); Vshell_file_name = build_string (sh ? sh : "/bin/sh"); @@ -1593,7 +1591,7 @@ init_callproc (void) Vshared_game_score_directory = Qnil; #else Vshared_game_score_directory = build_string (PATH_GAME); - if (NILP (Ffile_directory_p (Vshared_game_score_directory))) + if (NILP (Ffile_accessible_directory_p (Vshared_game_score_directory))) Vshared_game_score_directory = Qnil; #endif } diff --git a/src/charset.c b/src/charset.c index 6b999824dab..c9133c780e8 100644 --- a/src/charset.c +++ b/src/charset.c @@ -2293,7 +2293,7 @@ init_charset (void) { Lisp_Object tempdir; tempdir = Fexpand_file_name (build_string ("charsets"), Vdata_directory); - if (access (SSDATA (tempdir), 0) < 0) + if (! file_accessible_directory_p (SSDATA (tempdir))) { /* This used to be non-fatal (dir_warning), but it should not happen, and if it does sooner or later it will cause some diff --git a/src/conf_post.h b/src/conf_post.h index 66390ddf103..b1997e79081 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -178,6 +178,10 @@ extern void _DebPrint (const char *fmt, ...); #endif #endif +/* Tell gnulib to omit support for openat-related functions having a + first argument other than AT_FDCWD. */ +#define GNULIB_SUPPORT_ONLY_AT_FDCWD + #include #include diff --git a/src/fileio.c b/src/fileio.c index b9541e78838..572f6d8ef83 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -2425,15 +2425,7 @@ On Unix, this is a name starting with a `/' or a `~'. */) bool check_existing (const char *filename) { -#ifdef DOS_NT - /* The full emulation of Posix 'stat' is too expensive on - DOS/Windows, when all we want to know is whether the file exists. - So we use 'access' instead, which is much more lightweight. */ - return (access (filename, F_OK) >= 0); -#else - struct stat st; - return (stat (filename, &st) >= 0); -#endif + return faccessat (AT_FDCWD, filename, F_OK, AT_EACCESS) == 0; } /* Return true if file FILENAME exists and can be executed. */ @@ -2441,56 +2433,40 @@ check_existing (const char *filename) static bool check_executable (char *filename) { -#ifdef DOS_NT - struct stat st; - if (stat (filename, &st) < 0) - return 0; - return ((st.st_mode & S_IEXEC) != 0); -#else /* not DOS_NT */ -#ifdef HAVE_EUIDACCESS - return (euidaccess (filename, 1) >= 0); -#else - /* Access isn't quite right because it uses the real uid - and we really want to test with the effective uid. - But Unix doesn't give us a right way to do it. */ - return (access (filename, 1) >= 0); -#endif -#endif /* not DOS_NT */ + return faccessat (AT_FDCWD, filename, X_OK, AT_EACCESS) == 0; } -/* Return true if file FILENAME exists and can be written. */ +/* Return true if file FILENAME exists and can be accessed + according to AMODE, which should include W_OK. + On failure, return false and set errno. */ static bool -check_writable (const char *filename) +check_writable (const char *filename, int amode) { #ifdef MSDOS + /* FIXME: an faccessat implementation should be added to the + DOS/Windows ports and this #ifdef branch should be removed. */ struct stat st; if (stat (filename, &st) < 0) return 0; + errno = EPERM; return (st.st_mode & S_IWRITE || S_ISDIR (st.st_mode)); #else /* not MSDOS */ -#ifdef HAVE_EUIDACCESS - bool res = (euidaccess (filename, 2) >= 0); + bool res = faccessat (AT_FDCWD, filename, amode, AT_EACCESS) == 0; #ifdef CYGWIN - /* euidaccess may have returned failure because Cygwin couldn't + /* faccessat may have returned failure because Cygwin couldn't determine the file's UID or GID; if so, we return success. */ if (!res) { + int faccessat_errno = errno; struct stat st; if (stat (filename, &st) < 0) return 0; res = (st.st_uid == -1 || st.st_gid == -1); + errno = faccessat_errno; } #endif /* CYGWIN */ return res; -#else /* not HAVE_EUIDACCESS */ - /* Access isn't quite right because it uses the real uid - and we really want to test with the effective uid. - But Unix doesn't give us a right way to do it. - Opening with O_WRONLY could work for an ordinary file, - but would lose for directories. */ - return (access (filename, 2) >= 0); -#endif /* not HAVE_EUIDACCESS */ #endif /* not MSDOS */ } @@ -2547,9 +2523,6 @@ See also `file-exists-p' and `file-attributes'. */) { Lisp_Object absname; Lisp_Object handler; - int desc; - int flags; - struct stat statbuf; CHECK_STRING (filename); absname = Fexpand_file_name (filename, Qnil); @@ -2561,35 +2534,10 @@ See also `file-exists-p' and `file-attributes'. */) return call2 (handler, Qfile_readable_p, absname); absname = ENCODE_FILE (absname); - -#if defined (DOS_NT) || defined (macintosh) - /* Under MS-DOS, Windows, and Macintosh, open does not work for - directories. */ - if (access (SDATA (absname), 0) == 0) - return Qt; - return Qnil; -#else /* not DOS_NT and not macintosh */ - flags = O_RDONLY; -#ifdef O_NONBLOCK - /* Opening a fifo without O_NONBLOCK can wait. - We don't want to wait. But we don't want to mess wth O_NONBLOCK - except in the case of a fifo, on a system which handles it. */ - desc = stat (SSDATA (absname), &statbuf); - if (desc < 0) - return Qnil; - if (S_ISFIFO (statbuf.st_mode)) - flags |= O_NONBLOCK; -#endif - desc = emacs_open (SSDATA (absname), flags, 0); - if (desc < 0) - return Qnil; - emacs_close (desc); - return Qt; -#endif /* not DOS_NT and not macintosh */ + return (faccessat (AT_FDCWD, SSDATA (absname), R_OK, AT_EACCESS) == 0 + ? Qt : Qnil); } -/* Having this before file-symlink-p mysteriously caused it to be forgotten - on the RT/PC. */ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0, doc: /* Return t if file FILENAME can be written or created by you. */) (Lisp_Object filename) @@ -2607,14 +2555,15 @@ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0, return call2 (handler, Qfile_writable_p, absname); encoded = ENCODE_FILE (absname); - if (check_existing (SSDATA (encoded))) - return (check_writable (SSDATA (encoded)) - ? Qt : Qnil); + if (check_writable (SSDATA (encoded), W_OK)) + return Qt; + if (errno != ENOENT) + return Qnil; dir = Ffile_name_directory (absname); + eassert (!NILP (dir)); #ifdef MSDOS - if (!NILP (dir)) - dir = Fdirectory_file_name (dir); + dir = Fdirectory_file_name (dir); #endif /* MSDOS */ dir = ENCODE_FILE (dir); @@ -2622,10 +2571,9 @@ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0, /* The read-only attribute of the parent directory doesn't affect whether a file or directory can be created within it. Some day we should check ACLs though, which do affect this. */ - return (access (SDATA (dir), D_OK) < 0) ? Qnil : Qt; + return file_directory_p (SDATA (dir)) ? Qt : Qnil; #else - return (check_writable (!NILP (dir) ? SSDATA (dir) : "") - ? Qt : Qnil); + return check_writable (SSDATA (dir), W_OK | X_OK) ? Qt : Qnil; #endif } @@ -2703,8 +2651,7 @@ Symbolic links to directories count as directories. See `file-symlink-p' to distinguish symlinks. */) (Lisp_Object filename) { - register Lisp_Object absname; - struct stat st; + Lisp_Object absname; Lisp_Object handler; absname = expand_and_dir_to_file (filename, BVAR (current_buffer, directory)); @@ -2717,9 +2664,20 @@ See `file-symlink-p' to distinguish symlinks. */) absname = ENCODE_FILE (absname); - if (stat (SSDATA (absname), &st) < 0) - return Qnil; - return S_ISDIR (st.st_mode) ? Qt : Qnil; + return file_directory_p (SSDATA (absname)) ? Qt : Qnil; +} + +/* Return true if FILE is a directory or a symlink to a directory. */ +bool +file_directory_p (char const *file) +{ +#ifdef WINDOWSNT + /* This is cheaper than 'stat'. */ + return faccessat (AT_FDCWD, file, D_OK, AT_EACCESS) == 0; +#else + struct stat st; + return stat (file, &st) == 0 && S_ISDIR (st.st_mode); +#endif } DEFUN ("file-accessible-directory-p", Ffile_accessible_directory_p, @@ -2733,21 +2691,65 @@ if the directory so specified exists and really is a readable and searchable directory. */) (Lisp_Object filename) { + Lisp_Object absname; Lisp_Object handler; - bool tem; - struct gcpro gcpro1; + + CHECK_STRING (filename); + absname = Fexpand_file_name (filename, Qnil); /* If the file name has special constructs in it, call the corresponding file handler. */ - handler = Ffind_file_name_handler (filename, Qfile_accessible_directory_p); + handler = Ffind_file_name_handler (absname, Qfile_accessible_directory_p); if (!NILP (handler)) - return call2 (handler, Qfile_accessible_directory_p, filename); + return call2 (handler, Qfile_accessible_directory_p, absname); - GCPRO1 (filename); - tem = (NILP (Ffile_directory_p (filename)) - || NILP (Ffile_executable_p (filename))); - UNGCPRO; - return tem ? Qnil : Qt; + absname = ENCODE_FILE (absname); + return file_accessible_directory_p (SSDATA (absname)) ? Qt : Qnil; +} + +/* If FILE is a searchable directory or a symlink to a + searchable directory, return true. Otherwise return + false and set errno to an error number. */ +bool +file_accessible_directory_p (char const *file) +{ +#ifdef DOS_NT + /* There's no need to test whether FILE is searchable, as the + searchable/executable bit is invented on DOS_NT platforms. */ + return file_directory_p (file); +#else + /* On POSIXish platforms, use just one system call; this avoids a + race and is typically faster. */ + ptrdiff_t len = strlen (file); + char const *dir; + bool ok; + int saved_errno; + USE_SAFE_ALLOCA; + + /* Normally a file "FOO" is an accessible directory if "FOO/." exists. + There are three exceptions: "", "/", and "//". Leave "" alone, + as it's invalid. Append only "." to the other two exceptions as + "/" and "//" are distinct on some platforms, whereas "/", "///", + "////", etc. are all equivalent. */ + if (! len) + dir = file; + else + { + /* Just check for trailing '/' when deciding whether to append '/'. + That's simpler than testing the two special cases "/" and "//", + and it's a safe optimization here. */ + char *buf = SAFE_ALLOCA (len + 3); + memcpy (buf, file, len); + strcpy (buf + len, "/." + (file[len - 1] == '/')); + dir = buf; + } + + ok = check_existing (dir); + saved_errno = errno; + SAFE_FREE (); + errno = saved_errno; + return ok; +#endif } DEFUN ("file-regular-p", Ffile_regular_p, Sfile_regular_p, 1, 1, 0, @@ -3044,10 +3046,8 @@ Use the current time if TIMESTAMP is nil. TIMESTAMP is in the format of if (set_file_times (-1, SSDATA (encoded_absname), t, t)) { #ifdef MSDOS - struct stat st; - /* Setting times on a directory always fails. */ - if (stat (SSDATA (encoded_absname), &st) == 0 && S_ISDIR (st.st_mode)) + if (file_directory_p (SSDATA (encoded_absname))) return Qnil; #endif report_file_error ("Setting file times", Fcons (absname, Qnil)); diff --git a/src/lisp.h b/src/lisp.h index 72e38fa4653..67ae28a488f 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3202,6 +3202,8 @@ extern Lisp_Object close_file_unwind (Lisp_Object); extern Lisp_Object restore_point_unwind (Lisp_Object); extern _Noreturn void report_file_error (const char *, Lisp_Object); extern void internal_delete_file (Lisp_Object); +extern bool file_directory_p (const char *); +extern bool file_accessible_directory_p (const char *); extern void syms_of_fileio (void); extern Lisp_Object make_temp_name (Lisp_Object, bool); extern Lisp_Object Qdelete_file; diff --git a/src/lread.c b/src/lread.c index 3a82e0057e2..5859a2f85a9 100644 --- a/src/lread.c +++ b/src/lread.c @@ -1403,7 +1403,7 @@ Returns the file's name in absolute form, or nil if not found. If SUFFIXES is non-nil, it should be a list of suffixes to append to file name when searching. If non-nil, PREDICATE is used instead of `file-readable-p'. -PREDICATE can also be an integer to pass to the access(2) function, +PREDICATE can also be an integer to pass to the faccessat(2) function, in which case file-name-handlers are ignored. This function will normally skip directories, so if you want it to find directories, make sure the PREDICATE function returns `dir-ok' for them. */) @@ -1441,7 +1441,6 @@ static Lisp_Object Qdir_ok; int openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, Lisp_Object *storeptr, Lisp_Object predicate) { - int fd; ptrdiff_t fn_size = 100; char buf[100]; char *fn = buf; @@ -1496,7 +1495,6 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, Lisp_Object *sto { ptrdiff_t fnlen, lsuffix = SBYTES (XCAR (tail)); Lisp_Object handler; - bool exists; /* Concatenate path element/specified name with the suffix. If the directory starts with /:, remove that. */ @@ -1520,6 +1518,7 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, Lisp_Object *sto handler = Ffind_file_name_handler (string, Qfile_exists_p); if ((!NILP (handler) || !NILP (predicate)) && !NATNUMP (predicate)) { + bool exists; if (NILP (predicate)) exists = !NILP (Ffile_readable_p (string)); else @@ -1541,37 +1540,40 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, Lisp_Object *sto } else { -#ifndef WINDOWSNT - struct stat st; -#endif + int fd; const char *pfn; encoded_fn = ENCODE_FILE (string); pfn = SSDATA (encoded_fn); -#ifdef WINDOWSNT - exists = access (pfn, F_OK) == 0 && access (pfn, D_OK) < 0; -#else - exists = (stat (pfn, &st) == 0 && ! S_ISDIR (st.st_mode)); -#endif - if (exists) - { - /* Check that we can access or open it. */ - if (NATNUMP (predicate)) - fd = (((XFASTINT (predicate) & ~INT_MAX) == 0 - && access (pfn, XFASTINT (predicate)) == 0) - ? 1 : -1); - else - fd = emacs_open (pfn, O_RDONLY, 0); - if (fd >= 0) + /* Check that we can access or open it. */ + if (NATNUMP (predicate)) + fd = (((XFASTINT (predicate) & ~INT_MAX) == 0 + && (faccessat (AT_FDCWD, pfn, XFASTINT (predicate), + AT_EACCESS) + == 0) + && ! file_directory_p (pfn)) + ? 1 : -1); + else + { + struct stat st; + fd = emacs_open (pfn, O_RDONLY, 0); + if (0 <= fd + && (fstat (fd, &st) != 0 || S_ISDIR (st.st_mode))) { - /* We succeeded; return this descriptor and filename. */ - if (storeptr) - *storeptr = string; - UNGCPRO; - return fd; + emacs_close (fd); + fd = -1; } } + + if (fd >= 0) + { + /* We succeeded; return this descriptor and filename. */ + if (storeptr) + *storeptr = string; + UNGCPRO; + return fd; + } } } if (absolute) @@ -4087,9 +4089,8 @@ load_path_check (void) if (STRINGP (dirfile)) { dirfile = Fdirectory_file_name (dirfile); - if (access (SSDATA (dirfile), 0) < 0) - dir_warning ("Warning: Lisp directory `%s' does not exist.\n", - XCAR (path_tail)); + if (! file_accessible_directory_p (SSDATA (dirfile))) + dir_warning ("Lisp directory", XCAR (path_tail)); } } } @@ -4201,11 +4202,11 @@ init_lread (void) Lisp_Object tem, tem1; /* Add to the path the lisp subdir of the installation - dir, if it exists. Note: in out-of-tree builds, + dir, if it is accessible. Note: in out-of-tree builds, this directory is empty save for Makefile. */ tem = Fexpand_file_name (build_string ("lisp"), Vinstallation_directory); - tem1 = Ffile_exists_p (tem); + tem1 = Ffile_accessible_directory_p (tem); if (!NILP (tem1)) { if (NILP (Fmember (tem, Vload_path))) @@ -4222,10 +4223,10 @@ init_lread (void) Lisp dirs instead. */ Vload_path = nconc2 (Vload_path, dump_path); - /* Add leim under the installation dir, if it exists. */ + /* Add leim under the installation dir, if it is accessible. */ tem = Fexpand_file_name (build_string ("leim"), Vinstallation_directory); - tem1 = Ffile_exists_p (tem); + tem1 = Ffile_accessible_directory_p (tem); if (!NILP (tem1)) { if (NILP (Fmember (tem, Vload_path))) @@ -4237,7 +4238,7 @@ init_lread (void) { tem = Fexpand_file_name (build_string ("site-lisp"), Vinstallation_directory); - tem1 = Ffile_exists_p (tem); + tem1 = Ffile_accessible_directory_p (tem); if (!NILP (tem1)) { if (NILP (Fmember (tem, Vload_path))) @@ -4282,7 +4283,7 @@ init_lread (void) { tem = Fexpand_file_name (build_string ("site-lisp"), Vsource_directory); - tem1 = Ffile_exists_p (tem); + tem1 = Ffile_accessible_directory_p (tem); if (!NILP (tem1)) { if (NILP (Fmember (tem, Vload_path))) @@ -4338,21 +4339,28 @@ init_lread (void) Vloads_in_progress = Qnil; } -/* Print a warning, using format string FORMAT, that directory DIRNAME - does not exist. Print it on stderr and put it in *Messages*. */ +/* Print a warning that directory intended for use USE and with name + DIRNAME cannot be accessed. On entry, errno should correspond to + the access failure. Print the warning on stderr and put it in + *Messages*. */ void -dir_warning (const char *format, Lisp_Object dirname) +dir_warning (char const *use, Lisp_Object dirname) { - fprintf (stderr, format, SDATA (dirname)); + static char const format[] = "Warning: %s `%s': %s\n"; + int access_errno = errno; + fprintf (stderr, format, use, SSDATA (dirname), strerror (access_errno)); /* Don't log the warning before we've initialized!! */ if (initialized) { + char const *diagnostic = emacs_strerror (access_errno); USE_SAFE_ALLOCA; - char *buffer = SAFE_ALLOCA (SBYTES (dirname) - + strlen (format) - (sizeof "%s" - 1) + 1); - ptrdiff_t message_len = esprintf (buffer, format, SDATA (dirname)); + char *buffer = SAFE_ALLOCA (sizeof format - 3 * (sizeof "%s" - 1) + + strlen (use) + SBYTES (dirname) + + strlen (diagnostic)); + ptrdiff_t message_len = esprintf (buffer, format, use, SSDATA (dirname), + diagnostic); message_dolog (buffer, message_len, 0, STRING_MULTIBYTE (dirname)); SAFE_FREE (); } diff --git a/src/nsterm.m b/src/nsterm.m index 7ba1608268b..804ab825dee 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -4112,8 +4112,6 @@ ns_term_init (Lisp_Object display_name) color_file = Fexpand_file_name (build_string ("rgb.txt"), Fsymbol_value (intern ("data-directory"))); - if (NILP (Ffile_readable_p (color_file))) - fatal ("Could not find %s.\n", SDATA (color_file)); color_map = Fx_load_color_file (color_file); if (NILP (color_map)) diff --git a/src/process.c b/src/process.c index 43f0239d301..728abebe758 100644 --- a/src/process.c +++ b/src/process.c @@ -208,7 +208,7 @@ static EMACS_INT update_tick; #ifndef NON_BLOCKING_CONNECT #ifdef HAVE_SELECT #if defined (HAVE_GETPEERNAME) || defined (GNU_LINUX) -#if defined (O_NONBLOCK) || defined (O_NDELAY) +#if O_NONBLOCK || O_NDELAY #if defined (EWOULDBLOCK) || defined (EINPROGRESS) #define NON_BLOCKING_CONNECT #endif /* EWOULDBLOCK || EINPROGRESS */ @@ -655,7 +655,7 @@ allocate_pty (void) PTY_OPEN; #else /* no PTY_OPEN */ { -# ifdef O_NONBLOCK +# if O_NONBLOCK fd = emacs_open (pty_name, O_RDWR | O_NONBLOCK, 0); # else fd = emacs_open (pty_name, O_RDWR | O_NDELAY, 0); @@ -672,7 +672,7 @@ allocate_pty (void) #else sprintf (pty_name, "/dev/tty%c%x", c, i); #endif /* no PTY_TTY_NAME_SPRINTF */ - if (access (pty_name, 6) != 0) + if (faccessat (AT_FDCWD, pty_name, R_OK | W_OK, AT_EACCESS) != 0) { emacs_close (fd); # ifndef __sgi @@ -1624,7 +1624,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) #if ! defined (USG) || defined (USG_SUBTTY_WORKS) /* On most USG systems it does not work to open the pty's tty here, then close it and reopen it in the child. */ -#ifdef O_NOCTTY +#if O_NOCTTY /* Don't let this terminal become our controlling terminal (in case we don't have one). */ forkout = forkin = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0); @@ -1678,11 +1678,11 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) } #endif -#ifdef O_NONBLOCK +#if O_NONBLOCK fcntl (inchannel, F_SETFL, O_NONBLOCK); fcntl (outchannel, F_SETFL, O_NONBLOCK); #else -#ifdef O_NDELAY +#if O_NDELAY fcntl (inchannel, F_SETFL, O_NDELAY); fcntl (outchannel, F_SETFL, O_NDELAY); #endif @@ -1943,7 +1943,7 @@ create_pty (Lisp_Object process) #if ! defined (USG) || defined (USG_SUBTTY_WORKS) /* On most USG systems it does not work to open the pty's tty here, then close it and reopen it in the child. */ -#ifdef O_NOCTTY +#if O_NOCTTY /* Don't let this terminal become our controlling terminal (in case we don't have one). */ int forkout = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0); @@ -1963,11 +1963,11 @@ create_pty (Lisp_Object process) } #endif /* HAVE_PTYS */ -#ifdef O_NONBLOCK +#if O_NONBLOCK fcntl (inchannel, F_SETFL, O_NONBLOCK); fcntl (outchannel, F_SETFL, O_NONBLOCK); #else -#ifdef O_NDELAY +#if O_NDELAY fcntl (inchannel, F_SETFL, O_NDELAY); fcntl (outchannel, F_SETFL, O_NDELAY); #endif @@ -2927,7 +2927,7 @@ usage: (make-network-process &rest ARGS) */) { /* Don't support network sockets when non-blocking mode is not available, since a blocked Emacs is not useful. */ -#if !defined (O_NONBLOCK) && !defined (O_NDELAY) +#if !O_NONBLOCK && !O_NDELAY error ("Network servers not supported"); #else is_server = 1; @@ -3193,7 +3193,7 @@ usage: (make-network-process &rest ARGS) */) #ifdef NON_BLOCKING_CONNECT if (is_non_blocking_client) { -#ifdef O_NONBLOCK +#if O_NONBLOCK ret = fcntl (s, F_SETFL, O_NONBLOCK); #else ret = fcntl (s, F_SETFL, O_NDELAY); @@ -3410,10 +3410,10 @@ usage: (make-network-process &rest ARGS) */) chan_process[inch] = proc; -#ifdef O_NONBLOCK +#if O_NONBLOCK fcntl (inch, F_SETFL, O_NONBLOCK); #else -#ifdef O_NDELAY +#if O_NDELAY fcntl (inch, F_SETFL, O_NDELAY); #endif #endif @@ -4145,10 +4145,10 @@ server_accept_connection (Lisp_Object server, int channel) chan_process[s] = proc; -#ifdef O_NONBLOCK +#if O_NONBLOCK fcntl (s, F_SETFL, O_NONBLOCK); #else -#ifdef O_NDELAY +#if O_NDELAY fcntl (s, F_SETFL, O_NDELAY); #endif #endif @@ -4849,11 +4849,11 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, #endif /* ISC 4.1 defines both EWOULDBLOCK and O_NONBLOCK, and Emacs uses O_NONBLOCK, so what we get is EAGAIN. */ -#ifdef O_NONBLOCK +#if O_NONBLOCK else if (nread == -1 && errno == EAGAIN) ; #else -#ifdef O_NDELAY +#if O_NDELAY else if (nread == -1 && errno == EAGAIN) ; /* Note that we cannot distinguish between no input @@ -7348,7 +7348,7 @@ init_process_emacs (void) #ifdef HAVE_GETSOCKNAME ADD_SUBFEATURE (QCservice, Qt); #endif -#if defined (O_NONBLOCK) || defined (O_NDELAY) +#if O_NONBLOCK || O_NDELAY ADD_SUBFEATURE (QCserver, Qt); #endif diff --git a/src/sysdep.c b/src/sysdep.c index aa9d0f38c3c..a7f3de2f1b1 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -1287,7 +1287,7 @@ reset_sys_modes (struct tty_display_info *tty_out) old_fcntl_owner[fileno (tty_out->input)]); } #endif /* F_SETOWN */ -#ifdef O_NDELAY +#if O_NDELAY fcntl (fileno (tty_out->input), F_SETFL, fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NDELAY); #endif @@ -2384,12 +2384,12 @@ serial_open (char *port) fd = emacs_open ((char*) port, O_RDWR -#ifdef O_NONBLOCK +#if O_NONBLOCK | O_NONBLOCK #else | O_NDELAY #endif -#ifdef O_NOCTTY +#if O_NOCTTY | O_NOCTTY #endif , 0); diff --git a/src/term.c b/src/term.c index 578c701858f..96549290da5 100644 --- a/src/term.c +++ b/src/term.c @@ -2992,7 +2992,7 @@ init_tty (const char *name, const char *terminal_type, int must_succeed) int fd; FILE *file; -#ifdef O_IGNORE_CTTY +#if O_IGNORE_CTTY if (!ctty) /* Open the terminal device. Don't recognize it as our controlling terminal, and don't make it the controlling tty @@ -3023,7 +3023,7 @@ init_tty (const char *name, const char *terminal_type, int must_succeed) name); } -#ifndef O_IGNORE_CTTY +#if !O_IGNORE_CTTY if (!ctty) dissociate_if_controlling_tty (fd); #endif diff --git a/src/w32.c b/src/w32.c index 5ac1bc3eb7c..0e7da449b81 100644 --- a/src/w32.c +++ b/src/w32.c @@ -1597,7 +1597,7 @@ init_environment (char ** argv) see if it succeeds. But I think that's too much to ask. */ /* MSVCRT's _access crashes with D_OK. */ - if (tmp && sys_access (tmp, D_OK) == 0) + if (tmp && sys_faccessat (AT_FDCWD, tmp, D_OK, AT_EACCESS) == 0) { char * var = alloca (strlen (tmp) + 8); sprintf (var, "TMPDIR=%s", tmp); @@ -2714,10 +2714,16 @@ logon_network_drive (const char *path) long file names. */ int -sys_access (const char * path, int mode) +sys_faccessat (int dirfd, const char * path, int mode, int flags) { DWORD attributes; + if (dirfd != AT_FDCWD) + { + errno = EBADF; + return -1; + } + /* MSVCRT implementation of 'access' doesn't recognize D_OK, and its newer versions blow up when passed D_OK. */ path = map_w32_filename (path, NULL); @@ -2960,7 +2966,7 @@ sys_mktemp (char * template) { int save_errno = errno; p[0] = first_char[i]; - if (sys_access (template, 0) < 0) + if (sys_faccessat (AT_FDCWD, template, F_OK, AT_EACCESS) < 0) { errno = save_errno; return template; @@ -4011,7 +4017,7 @@ symlink (char const *filename, char const *linkname) { /* Non-absolute FILENAME is understood as being relative to LINKNAME's directory. We need to prepend that directory to - FILENAME to get correct results from sys_access below, since + FILENAME to get correct results from sys_faccessat below, since otherwise it will interpret FILENAME relative to the directory where the Emacs process runs. Note that make-symbolic-link always makes sure LINKNAME is a fully @@ -4025,10 +4031,10 @@ symlink (char const *filename, char const *linkname) strncpy (tem, linkfn, p - linkfn); tem[p - linkfn] = '\0'; strcat (tem, filename); - dir_access = sys_access (tem, D_OK); + dir_access = sys_faccessat (AT_FDCWD, tem, D_OK, AT_EACCESS); } else - dir_access = sys_access (filename, D_OK); + dir_access = sys_faccessat (AT_FDCWD, filename, D_OK, AT_EACCESS); /* Since Windows distinguishes between symlinks to directories and to files, we provide a kludgy feature: if FILENAME doesn't diff --git a/src/xrdb.c b/src/xrdb.c index 9d056a607e4..59b0876ebf8 100644 --- a/src/xrdb.c +++ b/src/xrdb.c @@ -41,7 +41,6 @@ along with GNU Emacs. If not, see . */ #ifdef HAVE_PWD_H #include #endif -#include #ifdef USE_MOTIF /* For Vdouble_click_time. */ @@ -50,7 +49,6 @@ along with GNU Emacs. If not, see . */ char *x_get_string_resource (XrmDatabase rdb, const char *name, const char *class); -static int file_p (const char *filename); /* X file search path processing. */ @@ -108,7 +106,7 @@ x_get_customization_string (XrmDatabase db, const char *name, database associated with display. (This is x_customization_string.) - Return the expanded file name if it exists and is readable, and + Return the resource database if its file was read successfully, and refers to %L only when the LANG environment variable is set, or otherwise provided by X. @@ -117,10 +115,11 @@ x_get_customization_string (XrmDatabase db, const char *name, Return NULL otherwise. */ -static char * -magic_file_p (const char *string, ptrdiff_t string_len, const char *class, - const char *escaped_suffix) +static XrmDatabase +magic_db (const char *string, ptrdiff_t string_len, const char *class, + const char *escaped_suffix) { + XrmDatabase db; char *lang = getenv ("LANG"); ptrdiff_t path_size = 100; @@ -217,14 +216,9 @@ magic_file_p (const char *string, ptrdiff_t string_len, const char *class, } path[path_len] = '\0'; - - if (! file_p (path)) - { - xfree (path); - return NULL; - } - - return path; + db = XrmGetFileDatabase (path); + xfree (path); + return db; } @@ -258,22 +252,11 @@ gethomedir (void) } -static int -file_p (const char *filename) -{ - struct stat status; - - return (access (filename, 4) == 0 /* exists and is readable */ - && stat (filename, &status) == 0 /* get the status */ - && (S_ISDIR (status.st_mode)) == 0); /* not a directory */ -} - - /* Find the first element of SEARCH_PATH which exists and is readable, after expanding the %-escapes. Return 0 if we didn't find any, and the path name of the one we found otherwise. */ -static char * +static XrmDatabase search_magic_path (const char *search_path, const char *class, const char *escaped_suffix) { @@ -286,18 +269,16 @@ search_magic_path (const char *search_path, const char *class, if (p > s) { - char *path = magic_file_p (s, p - s, class, escaped_suffix); - if (path) - return path; + XrmDatabase db = magic_db (s, p - s, class, escaped_suffix); + if (db) + return db; } else if (*p == ':') { - char *path; - - s = "%N%S"; - path = magic_file_p (s, strlen (s), class, escaped_suffix); - if (path) - return path; + static char const ns[] = "%N%S"; + XrmDatabase db = magic_db (ns, strlen (ns), class, escaped_suffix); + if (db) + return db; } if (*p == ':') @@ -312,21 +293,12 @@ search_magic_path (const char *search_path, const char *class, static XrmDatabase get_system_app (const char *class) { - XrmDatabase db = NULL; const char *path; - char *p; path = getenv ("XFILESEARCHPATH"); if (! path) path = PATH_X_DEFAULTS; - p = search_magic_path (path, class, 0); - if (p) - { - db = XrmGetFileDatabase (p); - xfree (p); - } - - return db; + return search_magic_path (path, class, 0); } @@ -340,35 +312,40 @@ get_fallback (Display *display) static XrmDatabase get_user_app (const char *class) { + XrmDatabase db = 0; const char *path; - char *file = 0; - char *free_it = 0; /* Check for XUSERFILESEARCHPATH. It is a path of complete file names, not directories. */ - if (((path = getenv ("XUSERFILESEARCHPATH")) - && (file = search_magic_path (path, class, 0))) + path = getenv ("XUSERFILESEARCHPATH"); + if (path) + db = search_magic_path (path, class, 0); + if (! db) + { /* Check for APPLRESDIR; it is a path of directories. In each, we have to search for LANG/CLASS and then CLASS. */ - || ((path = getenv ("XAPPLRESDIR")) - && ((file = search_magic_path (path, class, "/%L/%N")) - || (file = search_magic_path (path, class, "/%N")))) + path = getenv ("XAPPLRESDIR"); + if (path) + { + db = search_magic_path (path, class, "/%L/%N"); + if (!db) + db = search_magic_path (path, class, "/%N"); + } + } + if (! db) + { /* Check in the home directory. This is a bit of a hack; let's hope one's home directory doesn't contain any %-escapes. */ - || (free_it = gethomedir (), - ((file = search_magic_path (free_it, class, "%L/%N")) - || (file = search_magic_path (free_it, class, "%N"))))) - { - XrmDatabase db = XrmGetFileDatabase (file); - xfree (file); - xfree (free_it); - return db; + char *home = gethomedir (); + db = search_magic_path (home, class, "%L/%N"); + if (! db) + db = search_magic_path (home, class, "%N"); + xfree (home); } - xfree (free_it); - return NULL; + return db; } -- cgit v1.2.1 From 77731919f923a5fc81db60d19952776417a00037 Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Wed, 14 Nov 2012 15:13:33 +0400 Subject: * xdisp.c (echo_area_display, redisplay_internal): Omit redundant check whether frame_garbaged is set. --- src/ChangeLog | 5 +++++ src/xdisp.c | 12 ++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index a6b42e8a58c..99f3128b321 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2012-11-14 Dmitry Antipov + + * xdisp.c (echo_area_display, redisplay_internal): + Omit redundant check whether frame_garbaged is set. + 2012-11-14 Paul Eggert Use faccessat, not access, when checking file permissions (Bug#12632). diff --git a/src/xdisp.c b/src/xdisp.c index a74628db392..27d9fff0b7d 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -10816,8 +10816,7 @@ echo_area_display (int update_frame_p) #endif /* HAVE_WINDOW_SYSTEM */ /* Redraw garbaged frames. */ - if (frame_garbaged) - clear_garbaged_frames (); + clear_garbaged_frames (); if (!NILP (echo_area_buffer[0]) || minibuf_level == 0) { @@ -13104,8 +13103,7 @@ redisplay_internal (void) } /* Clear frames marked as garbaged. */ - if (frame_garbaged) - clear_garbaged_frames (); + clear_garbaged_frames (); /* Build menubar and tool-bar items. */ if (NILP (Vmemory_full)) @@ -13189,8 +13187,7 @@ redisplay_internal (void) /* If window configuration was changed, frames may have been marked garbaged. Clear them or we will experience surprises wrt scrolling. */ - if (frame_garbaged) - clear_garbaged_frames (); + clear_garbaged_frames (); } } else if (EQ (selected_window, minibuf_window) @@ -13213,8 +13210,7 @@ redisplay_internal (void) /* If window configuration was changed, frames may have been marked garbaged. Clear them or we will experience surprises wrt scrolling. */ - if (frame_garbaged) - clear_garbaged_frames (); + clear_garbaged_frames (); } -- cgit v1.2.1 From 730b2d8f6b5851dc462b79b8bd48068c1b9f1932 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Wed, 14 Nov 2012 18:41:43 +0200 Subject: Clean up w32 timer thread code in the hope of solving bug #12832. src/w32proc.c (timer_loop): Make sure SuspendThread and ResumeThread use the same value of thread handle. (start_timer_thread): If the timer thread exited (due to error), clean up by closing the two handles it used. Duplicate the caller thread's handle here, so it gets duplicated only once, when launching the timer thread. Set priority of the timer thread, not the caller thread. (getitimer): Don't duplicate the caller thread's handle here. --- src/ChangeLog | 12 ++++++++++++ src/w32proc.c | 38 ++++++++++++++++++++++++++------------ 2 files changed, 38 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index d72091c0ed6..9caa5113444 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,15 @@ +2012-11-14 Eli Zaretskii + + * w32proc.c (timer_loop): Make sure SuspendThread and ResumeThread + use the same value of thread handle. + (start_timer_thread): If the timer thread exited (due to error), + clean up by closing the two handles it used. Duplicate the caller + thread's handle here, so it gets duplicated only once, when + launching the timer thread. Set priority of the timer thread, not + the caller thread. + (getitimer): Don't duplicate the caller thread's handle here. + (Bug#12832) + 2012-11-13 Jan Djärv * nsterm.m (hold_event): Send SIGIO to make sure ns_read_socket is diff --git a/src/w32proc.c b/src/w32proc.c index adef7651b8c..e3c54fe5460 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -425,13 +425,14 @@ timer_loop (LPVOID arg) /* Simulate a signal delivered to the thread which installed the timer, by suspending that thread while the handler runs. */ - DWORD result = SuspendThread (itimer->caller_thread); + HANDLE th = itimer->caller_thread; + DWORD result = SuspendThread (th); if (result == (DWORD)-1) return 2; handler (sig); - ResumeThread (itimer->caller_thread); + ResumeThread (th); } /* Update expiration time and loop. */ @@ -556,6 +557,7 @@ static int start_timer_thread (int which) { DWORD exit_code; + HANDLE th; struct itimer_data *itimer = (which == ITIMER_REAL) ? &real_itimer : &prof_itimer; @@ -564,9 +566,29 @@ start_timer_thread (int which) && exit_code == STILL_ACTIVE) return 0; + /* Clean up after possibly exited thread. */ + if (itimer->timer_thread) + { + CloseHandle (itimer->timer_thread); + itimer->timer_thread = NULL; + } + if (itimer->caller_thread) + { + CloseHandle (itimer->caller_thread); + itimer->caller_thread = NULL; + } + /* Start a new thread. */ + if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (), + GetCurrentProcess (), &th, 0, FALSE, + DUPLICATE_SAME_ACCESS)) + { + errno = ESRCH; + return -1; + } itimer->terminate = 0; itimer->type = which; + itimer->caller_thread = th; /* Request that no more than 64KB of stack be reserved for this thread, to avoid reserving too much memory, which would get in the way of threads we start to wait for subprocesses. See also @@ -585,7 +607,7 @@ start_timer_thread (int which) /* This is needed to make sure that the timer thread running for profiling gets CPU as soon as the Sleep call terminates. */ if (which == ITIMER_PROF) - SetThreadPriority (itimer->caller_thread, THREAD_PRIORITY_TIME_CRITICAL); + SetThreadPriority (itimer->timer_thread, THREAD_PRIORITY_TIME_CRITICAL); return 0; } @@ -620,17 +642,9 @@ getitimer (int which, struct itimerval *value) itimer = (which == ITIMER_REAL) ? &real_itimer : &prof_itimer; - if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (), - GetCurrentProcess (), &itimer->caller_thread, 0, - FALSE, DUPLICATE_SAME_ACCESS)) - { - errno = ESRCH; - return -1; - } - ticks_now = w32_get_timer_time ((which == ITIMER_REAL) ? NULL - : itimer->caller_thread); + : GetCurrentThread ()); t_expire = &itimer->expire; t_reload = &itimer->reload; -- cgit v1.2.1 From 14f207289c224b3ad12fc8704c2183ef8fbcab28 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Wed, 14 Nov 2012 19:22:55 +0200 Subject: MS-Windows followup for 2012-11-14T04:55:41Z!eggert@cs.ucla.edu, regarding faccessat. nt/inc/unistd.h (faccessat): Add prototype. (AT_FDCWD, AT_EACCESS, AT_SYMLINK_NOFOLLOW): New macros; the first 2 moved from ms-w32.h. nt/inc/ms-w32.h (AT_FDCWD, AT_EACCESS, faccessat): Remove macros. src/w32.c (faccessat): Rename from sys_faccessat. (No need to use a different name, as the MS runtime does not have such a function, and probably never will.) All callers changed. Ignore DIRFD value if PATH is an absolute file name, to match Posix spec better. If AT_SYMLINK_NOFOLLOW is set in FLAGS, don't resolve symlinks. Fixes: debbugs:12632 --- src/ChangeLog | 9 +++++++++ src/w32.c | 30 +++++++++++++++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 99f3128b321..ec8f7e219f7 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,12 @@ +2012-11-14 Eli Zaretskii + + * w32.c (faccessat): Rename from sys_faccessat. (No need to use a + different name, as the MS runtime does not have such a function, + and probably never will.) All callers changed. Ignore DIRFD + value if PATH is an absolute file name, to match Posix spec + better. If AT_SYMLINK_NOFOLLOW is set in FLAGS, don't resolve + symlinks. + 2012-11-14 Dmitry Antipov * xdisp.c (echo_area_display, redisplay_internal): diff --git a/src/w32.c b/src/w32.c index 0e7da449b81..eb07e13a2fb 100644 --- a/src/w32.c +++ b/src/w32.c @@ -1597,7 +1597,7 @@ init_environment (char ** argv) see if it succeeds. But I think that's too much to ask. */ /* MSVCRT's _access crashes with D_OK. */ - if (tmp && sys_faccessat (AT_FDCWD, tmp, D_OK, AT_EACCESS) == 0) + if (tmp && faccessat (AT_FDCWD, tmp, D_OK, AT_EACCESS) == 0) { char * var = alloca (strlen (tmp) + 8); sprintf (var, "TMPDIR=%s", tmp); @@ -2708,17 +2708,15 @@ logon_network_drive (const char *path) WNetAddConnection2 (&resource, NULL, NULL, CONNECT_INTERACTIVE); } -/* Shadow some MSVC runtime functions to map requests for long filenames - to reasonable short names if necessary. This was originally added to - permit running Emacs on NT 3.1 on a FAT partition, which doesn't support - long file names. */ - +/* Emulate faccessat(2). */ int -sys_faccessat (int dirfd, const char * path, int mode, int flags) +faccessat (int dirfd, const char * path, int mode, int flags) { DWORD attributes; - if (dirfd != AT_FDCWD) + if (dirfd != AT_FDCWD + && !(IS_DIRECTORY_SEP (path[0]) + || IS_DEVICE_SEP (path[1]))) { errno = EBADF; return -1; @@ -2731,7 +2729,8 @@ sys_faccessat (int dirfd, const char * path, int mode, int flags) to get the attributes of its target file. Note: any symlinks in PATH elements other than the last one are transparently resolved by GetFileAttributes below. */ - if ((volume_info.flags & FILE_SUPPORTS_REPARSE_POINTS) != 0) + if ((volume_info.flags & FILE_SUPPORTS_REPARSE_POINTS) != 0 + && (flags & AT_SYMLINK_NOFOLLOW) == 0) path = chase_symlinks (path); if ((attributes = GetFileAttributes (path)) == -1) @@ -2781,6 +2780,11 @@ sys_faccessat (int dirfd, const char * path, int mode, int flags) return 0; } +/* Shadow some MSVC runtime functions to map requests for long filenames + to reasonable short names if necessary. This was originally added to + permit running Emacs on NT 3.1 on a FAT partition, which doesn't support + long file names. */ + int sys_chdir (const char * path) { @@ -2966,7 +2970,7 @@ sys_mktemp (char * template) { int save_errno = errno; p[0] = first_char[i]; - if (sys_faccessat (AT_FDCWD, template, F_OK, AT_EACCESS) < 0) + if (faccessat (AT_FDCWD, template, F_OK, AT_EACCESS) < 0) { errno = save_errno; return template; @@ -4017,7 +4021,7 @@ symlink (char const *filename, char const *linkname) { /* Non-absolute FILENAME is understood as being relative to LINKNAME's directory. We need to prepend that directory to - FILENAME to get correct results from sys_faccessat below, since + FILENAME to get correct results from faccessat below, since otherwise it will interpret FILENAME relative to the directory where the Emacs process runs. Note that make-symbolic-link always makes sure LINKNAME is a fully @@ -4031,10 +4035,10 @@ symlink (char const *filename, char const *linkname) strncpy (tem, linkfn, p - linkfn); tem[p - linkfn] = '\0'; strcat (tem, filename); - dir_access = sys_faccessat (AT_FDCWD, tem, D_OK, AT_EACCESS); + dir_access = faccessat (AT_FDCWD, tem, D_OK, AT_EACCESS); } else - dir_access = sys_faccessat (AT_FDCWD, filename, D_OK, AT_EACCESS); + dir_access = faccessat (AT_FDCWD, filename, D_OK, AT_EACCESS); /* Since Windows distinguishes between symlinks to directories and to files, we provide a kludgy feature: if FILENAME doesn't -- cgit v1.2.1 From bf20ea80f6331aaea18042a92928deb9db1b66f3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 14 Nov 2012 16:41:32 -0800 Subject: * eval.c (mark_backtrace) [BYTE_MARK_STACK]: Remove stray '*'. This follows up on the 2012-09-29 patch that removed indirection for the 'function' field. Reported by Sergey Vinokurov in . --- src/ChangeLog | 7 +++++++ src/eval.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index ec8f7e219f7..d309931e8e6 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,10 @@ +2012-11-15 Paul Eggert + + * eval.c (mark_backtrace) [BYTE_MARK_STACK]: Remove stray '*'. + This follows up on the 2012-09-29 patch that removed indirection + for the 'function' field. Reported by Sergey Vinokurov in + . + 2012-11-14 Eli Zaretskii * w32.c (faccessat): Rename from sys_faccessat. (No need to use a diff --git a/src/eval.c b/src/eval.c index dcd48cb7250..c9f27ea8d77 100644 --- a/src/eval.c +++ b/src/eval.c @@ -3369,7 +3369,7 @@ mark_backtrace (void) for (backlist = backtrace_list; backlist; backlist = backlist->next) { - mark_object (*backlist->function); + mark_object (backlist->function); if (backlist->nargs == UNEVALLED || backlist->nargs == MANY) /* FIXME: Can this happen? */ -- cgit v1.2.1 From bde3c6c0f79ab814e12ea0f04b06625f91f5cd52 Mon Sep 17 00:00:00 2001 From: Glenn Morris Date: Wed, 14 Nov 2012 23:30:46 -0800 Subject: Fixes related to face underlining * lisp/faces.el (face-underline-p): Doc fix. Handle :underline being things other than `t' (a string, a list). (face-inverse-video-p): Doc fix. (set-face-underline): Rename it back from set-face-underline-p. Doc fix. Allow interactive input of values other than t. (read-face-attribute): Apply formatting to :underline, since like :box and :stipple it can take list values. * doc/lispref/display.texi (Face Attributes): Fix :underline COLOR description. (Attribute Functions): Update for set-face-underline rename. Tweak descriptions of face-underline-p, face-inverse-video-p. * etc/NEWS: Related edit. --- src/xfaces.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src') diff --git a/src/xfaces.c b/src/xfaces.c index 221387c4b6d..5eda6dca6da 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -2906,6 +2906,12 @@ FRAME 0 means change the face on all frames, and change the default Lisp_Object key, val, list; list = value; + /* FIXME? This errs on the side of acceptance. Eg it accepts: + (defface foo '((t :underline 'foo) "doc") + Maybe this is intentional, maybe it isn't. + Non-nil symbols other than t are not documented as being valid. + Eg compare with inverse-video, which explicitly rejects them. + */ valid_p = 1; while (!NILP (CAR_SAFE(list))) @@ -5727,6 +5733,8 @@ realize_x_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]) face->underline_defaulted_p = 1; face->underline_type = FACE_UNDER_LINE; + /* FIXME? This is also not robust about checking the precise form. + See comments in Finternal_set_lisp_face_attribute. */ while (CONSP (underline)) { Lisp_Object keyword, value; -- cgit v1.2.1 From b72c161c5bda1836b0a86ceba1bd968abd00ff1a Mon Sep 17 00:00:00 2001 From: Juanma Barranquero Date: Thu, 15 Nov 2012 17:21:50 +0100 Subject: src/makefile.w32-in: Update dependencies. --- src/ChangeLog | 5 +++++ src/makefile.w32-in | 2 ++ 2 files changed, 7 insertions(+) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index d309931e8e6..8caaa2f68e0 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2012-11-15 Juanma Barranquero + + * makefile.w32-in ($(BLD)/dispnew.$(O), $(BLD)/emacs.$(O)): + Update dependencies. + 2012-11-15 Paul Eggert * eval.c (mark_backtrace) [BYTE_MARK_STACK]: Remove stray '*'. diff --git a/src/makefile.w32-in b/src/makefile.w32-in index f5cab34d7dc..69fd6857f86 100644 --- a/src/makefile.w32-in +++ b/src/makefile.w32-in @@ -737,6 +737,7 @@ $(BLD)/dispnew.$(O) : \ $(SRC)/termchar.h \ $(SRC)/w32.h \ $(NT_INC)/unistd.h \ + $(GNU_LIB)/fpending.h \ $(BUFFER_H) \ $(CHARACTER_H) \ $(CONFIG_H) \ @@ -802,6 +803,7 @@ $(BLD)/emacs.$(O) : \ $(SRC)/w32select.h \ $(NT_INC)/sys/file.h \ $(NT_INC)/unistd.h \ + $(GNU_LIB)/close-stream.h \ $(GNU_LIB)/ignore-value.h \ $(ATIMER_H) \ $(BUFFER_H) \ -- cgit v1.2.1 From 5c2a71483b029100aabf5d64717120b31f4d6fa4 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Thu, 15 Nov 2012 12:17:23 -0500 Subject: * src/eval.c (Finteractive_p): Revert lexbind-merge mishap. --- src/ChangeLog | 16 ++++++++++------ src/eval.c | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 9caa5113444..d2e7a96f275 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,7 @@ +2012-11-15 Stefan Monnier + + * eval.c (Finteractive_p): Revert lexbind-merge mishap. + 2012-11-14 Eli Zaretskii * w32proc.c (timer_loop): Make sure SuspendThread and ResumeThread @@ -28,16 +32,16 @@ * window.c (Fsplit_window_internal): Set combination limit of new parent window to t iff Vwindow_combination_limit is t; fixing a regression introduced with the change from 2012-09-22. - (Fwindow_combination_limit, Fset_window_combination_limit): Fix - doc-strings. + (Fwindow_combination_limit, Fset_window_combination_limit): + Fix doc-strings. 2012-11-06 Eli Zaretskii * xdisp.c (try_scrolling): Fix correction of aggressive-scroll amount when the scroll margins are too large. When scrolling backwards in the buffer, give up if cannot reach point or the - scroll margin within a reasonable number of screen lines. Fixes - point position in window under scroll-up/down-aggressively when + scroll margin within a reasonable number of screen lines. + Fixes point position in window under scroll-up/down-aggressively when point is positioned many lines beyond the window top/bottom. (Bug#12811) @@ -118,8 +122,8 @@ 2012-10-29 Daniel Colascione - * cygw32.h, cygw32.c (Qutf_16le, from_unicode, to_unicode): In - preparation for fixing bug#12739, move these functions from + * cygw32.h, cygw32.c (Qutf_16le, from_unicode, to_unicode): + In preparation for fixing bug#12739, move these functions from here... * coding.h, coding.c: ... to here, and compile them only when diff --git a/src/eval.c b/src/eval.c index 975204da017..58fa92cd7b5 100644 --- a/src/eval.c +++ b/src/eval.c @@ -508,7 +508,7 @@ spec that specifies non-nil unconditionally (such as \"p\"); or (ii) use `called-interactively-p'. */) (void) { - return interactive_p () ? Qt : Qnil; + return (INTERACTIVE && interactive_p ()) ? Qt : Qnil; } -- cgit v1.2.1 From d56f2e49b2353336e853025219440c3e1572524e Mon Sep 17 00:00:00 2001 From: Glenn Morris Date: Thu, 15 Nov 2012 21:40:54 -0500 Subject: * src/editfns.c (Fmessage): Mention message-log-max. (Bug#12849) --- src/ChangeLog | 4 ++++ src/editfns.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index d2e7a96f275..b218e42b3f2 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,7 @@ +2012-11-16 Glenn Morris + + * editfns.c (Fmessage): Mention message-log-max. (Bug#12849) + 2012-11-15 Stefan Monnier * eval.c (Finteractive_p): Revert lexbind-merge mishap. diff --git a/src/editfns.c b/src/editfns.c index c5d4ed295ab..8122ffdd0d4 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -3434,8 +3434,8 @@ static ptrdiff_t message_length; DEFUN ("message", Fmessage, Smessage, 1, MANY, 0, doc: /* Display a message at the bottom of the screen. -The message also goes into the `*Messages*' buffer. -\(In keyboard macros, that's all it does.) +The message also goes into the `*Messages*' buffer, if `message-log-max' +is non-nil. (In keyboard macros, that's all it does.) Return the message. The first argument is a format control string, and the rest are data -- cgit v1.2.1 From 3d082a269ece18058ed82957f8a056822b39789e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 16 Nov 2012 07:29:22 -0800 Subject: Remove no-longer-used pty_max_bytes variable. * configure.ac (fpathconf): Remove unnecessary check. * admin/CPP-DEFINES (HAVE_FPATHCONF): Remove. * src/process.c (pty_max_bytes): Remove; unused. (send_process): Do not set it. --- src/ChangeLog | 6 ++++++ src/process.c | 16 ---------------- 2 files changed, 6 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 8caaa2f68e0..c9c754f8677 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,9 @@ +2012-11-16 Paul Eggert + + Remove no-longer-used pty_max_bytes variable. + * process.c (pty_max_bytes): Remove; unused. + (send_process): Do not set it. + 2012-11-15 Juanma Barranquero * makefile.w32-in ($(BLD)/dispnew.$(O), $(BLD)/emacs.$(O)): diff --git a/src/process.c b/src/process.c index 728abebe758..785282fba36 100644 --- a/src/process.c +++ b/src/process.c @@ -340,9 +340,6 @@ static struct sockaddr_and_len { #define DATAGRAM_CONN_P(proc) (0) #endif -/* Maximum number of bytes to send to a pty without an eof. */ -static int pty_max_bytes; - /* These setters are used only in this file, so they can be private. */ static void pset_buffer (struct Lisp_Process *p, Lisp_Object val) @@ -5532,19 +5529,6 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len, buf = SSDATA (object); } - if (pty_max_bytes == 0) - { -#if defined (HAVE_FPATHCONF) && defined (_PC_MAX_CANON) - pty_max_bytes = fpathconf (p->outfd, _PC_MAX_CANON); - if (pty_max_bytes < 0) - pty_max_bytes = 250; -#else - pty_max_bytes = 250; -#endif - /* Deduct one, to leave space for the eof. */ - pty_max_bytes--; - } - /* If there is already data in the write_queue, put the new data in the back of queue. Otherwise, ignore it. */ if (!NILP (p->write_queue)) -- cgit v1.2.1 From a631d0e04747884855aa460cb903d1fd2ff106f4 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 17 Nov 2012 07:15:49 -0800 Subject: Fix problems in ns port found by static checking. * nsterm.m: Include , for pthread_mutex_lock etc. (hold_event, setPosition:portion:whole:): Send SIGIO only to self, not to process group. (ns_select): Use emacs_write, not write, as that's more robust in the presence of signals. (fd_handler:): Check for read errors. --- src/ChangeLog | 10 ++++++++++ src/nsterm.m | 22 ++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 695fbab5813..1194fe099fa 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,13 @@ +2012-11-17 Paul Eggert + + Fix problems in ns port found by static checking. + * nsterm.m: Include , for pthread_mutex_lock etc. + (hold_event, setPosition:portion:whole:): Send SIGIO only to self, + not to process group. + (ns_select): Use emacs_write, not write, as that's more robust + in the presence of signals. + (fd_handler:): Check for read errors. + 2012-11-16 Glenn Morris * editfns.c (Fmessage): Mention message-log-max. (Bug#12849) diff --git a/src/nsterm.m b/src/nsterm.m index 7c66708e7cb..3640ac0c5e8 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -31,6 +31,7 @@ GNUstep port and post-20 update by Adrian Robert (arobert@cogsci.ucsd.edu) #include #include +#include #include #include #include @@ -331,7 +332,7 @@ hold_event (struct input_event *event) hold_event_q.q[hold_event_q.nr++] = *event; /* Make sure ns_read_socket is called, i.e. we have input. */ - kill (0, SIGIO); + raise (SIGIO); } static Lisp_Object @@ -3389,7 +3390,7 @@ ns_read_socket (struct terminal *terminal, struct input_event *hold_quit) if ([NSApp modalWindow] != nil) return -1; - if (hold_event_q.nr > 0) + if (hold_event_q.nr > 0) { int i; for (i = 0; i < hold_event_q.nr; ++i) @@ -3504,7 +3505,7 @@ ns_select (int nfds, fd_set *readfds, fd_set *writefds, /* Inform fd_handler that select should be called */ c = 'g'; - write (selfds[1], &c, 1); + emacs_write (selfds[1], &c, 1); } else if (nr == 0 && timeout) { @@ -3537,7 +3538,7 @@ ns_select (int nfds, fd_set *readfds, fd_set *writefds, if (nr > 0 && readfds) { c = 's'; - write (selfds[1], &c, 1); + emacs_write (selfds[1], &c, 1); } unblock_input (); @@ -4576,11 +4577,8 @@ not_in_argv (NSString *arg) FD_SET (selfds[0], &fds); result = select (selfds[0]+1, &fds, NULL, NULL, NULL); - if (result > 0) - { - read (selfds[0], &c, 1); - if (c == 'g') waiting = 0; - } + if (result > 0 && read (selfds[0], &c, 1) == 1 && c == 'g') + waiting = 0; } else { @@ -4620,8 +4618,8 @@ not_in_argv (NSString *arg) { if (FD_ISSET (selfds[0], &readfds)) { - read (selfds[0], &c, 1); - if (c == 's') waiting = 1; + if (read (selfds[0], &c, 1) == 1 && c == 's') + waiting = 1; } else { @@ -6696,7 +6694,7 @@ not_in_argv (NSString *arg) /* Events may come here even if the event loop is not running. If we don't enter the event loop, the scroll bar will not update. So send SIGIO to ourselves. */ - if (apploopnr == 0) kill (0, SIGIO); + if (apploopnr == 0) raise (SIGIO); return self; } -- cgit v1.2.1 From 22bae83fa8c432780fe20202a660aa8c84f3087a Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 17 Nov 2012 18:46:45 +0200 Subject: Fix bug #12829 with aborts on MS-Windows when several child processes die. nt/inc/sys/wait.h: New file, with prototype of waitpid and definitions of macros it needs. nt/inc/ms-w32.h (wait): Don't define, 'wait' is not used anymore. (sys_wait): Remove prototype. nt/config.nt (HAVE_SYS_WAIT_H): Define to 1. src/w32proc.c (create_child): Don't clip the PID of the child process to fit into an Emacs integer, as this is no longer a restriction. (waitpid): Rename from sys_wait. Emulate a Posix 'waitpid' by reaping only the process specified by PID argument, if that is positive. Use PID instead of dead_child to know which process to reap. Wait for the child to die only if WNOHANG is not in OPTIONS. (sys_select): Don't set dead_child. src/sysdep.c (wait_for_termination_1): Remove the WINDOWSNT portion, as it is no longer needed. src/process.c (waitpid, WUNTRACED) [!WNOHANG]: Remove definitions, no longer needed. (record_child_status_change): Remove the setting of record_at_most_one_child for the !WNOHANG case. --- src/ChangeLog | 20 +++++++++ src/process.c | 20 --------- src/sysdep.c | 7 +--- src/w32proc.c | 127 +++++++++++++++++++++++++++++++++++++++++----------------- 4 files changed, 112 insertions(+), 62 deletions(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 1194fe099fa..45d48ba41cc 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,23 @@ +2012-11-17 Eli Zaretskii + + * w32proc.c (create_child): Don't clip the PID of the child + process to fit into an Emacs integer, as this is no longer a + restriction. + (waitpid): Rename from sys_wait. Emulate a Posix 'waitpid' by + reaping only the process specified by PID argument, if that is + positive. Use PID instead of dead_child to know which process to + reap. Wait for the child to die only if WNOHANG is not in + OPTIONS. + (sys_select): Don't set dead_child. + + * sysdep.c (wait_for_termination_1): Remove the WINDOWSNT portion, + as it is no longer needed. + + * process.c (waitpid, WUNTRACED) [!WNOHANG]: Remove definitions, + no longer needed. + (record_child_status_change): Remove the setting of + record_at_most_one_child for the !WNOHANG case. + 2012-11-17 Paul Eggert Fix problems in ns port found by static checking. diff --git a/src/process.c b/src/process.c index 785282fba36..5fe6a6540f3 100644 --- a/src/process.c +++ b/src/process.c @@ -130,18 +130,6 @@ extern int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *, EMACS_TIME *, void *); #endif -/* This is for DOS_NT ports. FIXME: Remove this old portability cruft - by having DOS_NT ports implement waitpid instead of wait. Nowadays - POSIXish hosts all define waitpid, WNOHANG, and WUNTRACED, as these - have been standard since POSIX.1-1988. */ -#ifndef WNOHANG -# undef waitpid -# define waitpid(pid, status, options) wait (status) -#endif -#ifndef WUNTRACED -# define WUNTRACED 0 -#endif - /* Work around GCC 4.7.0 bug with strict overflow checking; see . These lines can be removed once the GCC bug is fixed. */ @@ -6295,17 +6283,9 @@ record_child_status_change (pid_t pid, int w) { #ifdef SIGCHLD -# ifdef WNOHANG /* On POSIXish hosts, record at most one child only if we already know one child that has exited. */ bool record_at_most_one_child = 0 <= pid; -# else - /* On DOS_NT (the only porting target that lacks WNOHANG), - record the status of at most one child process, since the SIGCHLD - handler must return right away. If any more processes want to - signal us, we will get another signal. */ - bool record_at_most_one_child = 1; -# endif Lisp_Object tail; diff --git a/src/sysdep.c b/src/sysdep.c index a7f3de2f1b1..06dc41b511e 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -289,10 +289,6 @@ wait_for_termination_1 (pid_t pid, int interruptible) { while (1) { -#ifdef WINDOWSNT - wait (0); - break; -#else /* not WINDOWSNT */ int status; int wait_result = waitpid (pid, &status, 0); if (wait_result < 0) @@ -306,7 +302,8 @@ wait_for_termination_1 (pid_t pid, int interruptible) break; } -#endif /* not WINDOWSNT */ + /* Note: the MS-Windows emulation of waitpid calls QUIT + internally. */ if (interruptible) QUIT; } diff --git a/src/w32proc.c b/src/w32proc.c index 10dd23003b8..fd6a498290a 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -789,7 +789,6 @@ alarm (int seconds) /* Child process management list. */ int child_proc_count = 0; child_process child_procs[ MAX_CHILDREN ]; -child_process *dead_child = NULL; static DWORD WINAPI reader_thread (void *arg); @@ -1042,9 +1041,6 @@ create_child (char *exe, char *cmdline, char *env, int is_gui_app, if (cp->pid < 0) cp->pid = -cp->pid; - /* pid must fit in a Lisp_Int */ - cp->pid = cp->pid & INTMASK; - *pPid = cp->pid; return TRUE; @@ -1120,55 +1116,110 @@ reap_subprocess (child_process *cp) delete_child (cp); } -/* Wait for any of our existing child processes to die - When it does, close its handle - Return the pid and fill in the status if non-NULL. */ +/* Wait for a child process specified by PID, or for any of our + existing child processes (if PID is nonpositive) to die. When it + does, close its handle. Return the pid of the process that died + and fill in STATUS if non-NULL. */ -int -sys_wait (int *status) +pid_t +waitpid (pid_t pid, int *status, int options) { DWORD active, retval; int nh; - int pid; child_process *cp, *cps[MAX_CHILDREN]; HANDLE wait_hnd[MAX_CHILDREN]; + DWORD timeout_ms; + int dont_wait = (options & WNOHANG) != 0; nh = 0; - if (dead_child != NULL) + /* According to Posix: + + PID = -1 means status is requested for any child process. + + PID > 0 means status is requested for a single child process + whose pid is PID. + + PID = 0 means status is requested for any child process whose + process group ID is equal to that of the calling process. But + since Windows has only a limited support for process groups (only + for console processes and only for the purposes of passing + Ctrl-BREAK signal to them), and since we have no documented way + of determining whether a given process belongs to our group, we + treat 0 as -1. + + PID < -1 means status is requested for any child process whose + process group ID is equal to the absolute value of PID. Again, + since we don't support process groups, we treat that as -1. */ + if (pid > 0) { - /* We want to wait for a specific child */ - wait_hnd[nh] = dead_child->procinfo.hProcess; - cps[nh] = dead_child; - if (!wait_hnd[nh]) emacs_abort (); - nh++; - active = 0; - goto get_result; + int our_child = 0; + + /* We are requested to wait for a specific child. */ + for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) + { + /* Some child_procs might be sockets; ignore them. Also + ignore subprocesses whose output is not yet completely + read. */ + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && cp->pid == pid) + { + our_child = 1; + break; + } + } + if (our_child) + { + if (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + else if (dont_wait) + { + /* PID specifies our subprocess, but its status is not + yet available. */ + return 0; + } + } + if (nh == 0) + { + /* No such child process, or nothing to wait for, so fail. */ + errno = ECHILD; + return -1; + } } else { for (cp = child_procs + (child_proc_count-1); cp >= child_procs; cp--) - /* some child_procs might be sockets; ignore them */ - if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess - && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) - { - wait_hnd[nh] = cp->procinfo.hProcess; - cps[nh] = cp; - nh++; - } + { + if (CHILD_ACTIVE (cp) + && cp->procinfo.hProcess + && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0)) + { + wait_hnd[nh] = cp->procinfo.hProcess; + cps[nh] = cp; + nh++; + } + } + if (nh == 0) + { + /* Nothing to wait on, so fail. */ + errno = ECHILD; + return -1; + } } - if (nh == 0) - { - /* Nothing to wait on, so fail */ - errno = ECHILD; - return -1; - } + if (dont_wait) + timeout_ms = 0; + else + timeout_ms = 1000; /* check for quit about once a second. */ do { - /* Check for quit about once a second. */ QUIT; - active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000); + active = WaitForMultipleObjects (nh, wait_hnd, FALSE, timeout_ms); } while (active == WAIT_TIMEOUT); if (active == WAIT_FAILED) @@ -1198,8 +1249,10 @@ get_result: } if (retval == STILL_ACTIVE) { - /* Should never happen */ + /* Should never happen. */ DebPrint (("Wait.WaitForMultipleObjects returned an active process\n")); + if (pid > 0 && dont_wait) + return 0; errno = EINVAL; return -1; } @@ -1213,6 +1266,8 @@ get_result: else retval <<= 8; + if (pid > 0 && active != 0) + emacs_abort (); cp = cps[active]; pid = cp->pid; #ifdef FULL_DEBUG @@ -2001,9 +2056,7 @@ count_children: DebPrint (("select calling SIGCHLD handler for pid %d\n", cp->pid)); #endif - dead_child = cp; sig_handlers[SIGCHLD] (SIGCHLD); - dead_child = NULL; } } else if (fdindex[active] == -1) -- cgit v1.2.1 From 6ad30855c02908fdd99d9b11943719e185e65ee3 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 17 Nov 2012 18:52:48 +0200 Subject: Fix MS-Windows emulation of 'faccessat' wrt directories. src/w32.c (faccessat): Pretend that directories have the execute bit set. Emacs expects that, e.g., in files.el:cd-absolute. --- src/ChangeLog | 3 +++ src/w32.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/ChangeLog b/src/ChangeLog index 45d48ba41cc..df8bf602afe 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,8 @@ 2012-11-17 Eli Zaretskii + * w32.c (faccessat): Pretend that directories have the execute bit + set. Emacs expects that, e.g., in files.el:cd-absolute. + * w32proc.c (create_child): Don't clip the PID of the child process to fit into an Emacs integer, as this is no longer a restriction. diff --git a/src/w32.c b/src/w32.c index eb07e13a2fb..46433626802 100644 --- a/src/w32.c +++ b/src/w32.c @@ -2762,7 +2762,8 @@ faccessat (int dirfd, const char * path, int mode, int flags) } return -1; } - if ((mode & X_OK) != 0 && !is_exec (path)) + if ((mode & X_OK) != 0 + && !(is_exec (path) || (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0)) { errno = EACCES; return -1; -- cgit v1.2.1