Replace strerror/sys_errlist after fork with our own errors (#8234)

* Remove safe_strerror, safe_perror and safe_append

This no longer works on new glibcs because they removed sys_errlist.

So just hardcode the relevant errno messages (and phrase them better).

Fixes #4183.

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>

cmake/ConfigureChecks.cmake

@@ -108,7 +108,6 @@ check_struct_has_member("struct stat" st_mtimespec.tv_nsec "sys/stat.h"
     HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC LANGUAGE CXX)
 check_struct_has_member("struct stat" st_mtim.tv_nsec "sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC
     LANGUAGE CXX)
-check_cxx_symbol_exists(sys_errlist stdio.h HAVE_SYS_ERRLIST)
 check_include_file_cxx(sys/ioctl.h HAVE_SYS_IOCTL_H)
 check_include_file_cxx(sys/select.h HAVE_SYS_SELECT_H)
 check_include_files("sys/types.h;sys/sysctl.h" HAVE_SYS_SYSCTL_H)
@@ -143,8 +142,6 @@ endif()
 list(APPEND WCSTOD_L_INCLUDES "wchar.h")
 check_cxx_symbol_exists(wcstod_l "${WCSTOD_L_INCLUDES}" HAVE_WCSTOD_L)
 
-check_cxx_symbol_exists(_sys_errs stdlib.h HAVE__SYS__ERRS)
-
 cmake_push_check_state()
 set(CMAKE_EXTRA_INCLUDE_FILES termios.h sys/ioctl.h)
 check_type_size("struct winsize" STRUCT_WINSIZE LANGUAGE CXX)

config_cmake.h.in

@@ -91,9 +91,6 @@
 /* Define to 1 if `st_mtim.tv_nsec' is a member of `struct stat'. */
 #cmakedefine HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 1
 
-/* Define to 1 if the sys_errlist array is available. */
-#cmakedefine HAVE_SYS_ERRLIST 1
-
 /* Define to 1 if you have the <sys/ioctl.h> header file. */
 #cmakedefine HAVE_SYS_IOCTL_H 1
 
@@ -136,9 +133,6 @@
 /* Define to 1 if std::make_unique is available. */
 #cmakedefine HAVE_STD__MAKE_UNIQUE 1
 
-/* Define to 1 if the _sys_errs array is available. */
-#cmakedefine HAVE__SYS__ERRS 1
-
 /* Define to 1 to disable ncurses macros that conflict with the STL */
 #define NCURSES_NOMACROS 1
 

src/postfork.cpp

@@ -68,7 +68,30 @@ void report_setpgid_error(int err, bool is_parent, pid_t desired_pgid, const job
     }
 
     errno = err;
-    safe_perror("setpgid");
+    switch (errno) {
+        case EACCES: {
+            FLOGF_SAFE(error, "setpgid: Process %s has already exec'd", pid_buff);
+            break;
+        }
+        case EINVAL: {
+            FLOGF_SAFE(error, "setpgid: pgid %s unsupported", getpgid_buff);
+            break;
+        }
+        case EPERM: {
+            FLOGF_SAFE(error, "setpgid: Process %s is a session leader or pgid %s does not match", pid_buff, getpgid_buff);
+            break;
+        }
+        case ESRCH: {
+            FLOGF_SAFE(error, "setpgid: Process ID %s does not match", pid_buff);
+            break;
+        }
+        default: {
+            char errno_buff[64];
+            format_long_safe(errno_buff, errno);
+            FLOGF_SAFE(error, "setpgid: Unknown error number %s", errno_buff);
+            break;
+        }
+    }
 }
 
 int execute_setpgid(pid_t pid, pid_t pgroup, bool is_parent) {
@@ -200,7 +223,26 @@ pid_t execute_fork() {
         }
     }
 
-    safe_perror("fork");
+    // These are all the errno numbers for fork() I can find.
+    // Also ENOSYS, but I doubt anyone is running
+    // fish on a platform without an MMU.
+    switch (errno) {
+        case EAGAIN: {
+            // We should have retried these already?
+            FLOGF_SAFE(error, "fork: Out of resources. Check RLIMIT_NPROC and pid_max.");
+            break;
+        }
+        case ENOMEM: {
+            FLOGF_SAFE(error, "fork: Out of memory.");
+            break;
+        }
+        default: {
+            char errno_buff[64];
+            format_long_safe(errno_buff, errno);
+            FLOGF_SAFE(error, "fork: Unknown error number %s", errno_buff);
+            break;
+        }
+    }
     FATAL_EXIT();
     return 0;
 }
@@ -370,12 +412,11 @@ void safe_report_exec_error(int err, const char *actual_cmd, const char *const *
         }
 
         case ENOEXEC: {
-            const char *err_text = safe_strerror(err);
             FLOGF_SAFE(
                 exec,
-                "%s. The file '%s' is marked as an executable but could not be run by the "
+                "The file '%s' is marked as an executable but could not be run by the "
                 "operating system.",
-                err_text, actual_cmd);
+                actual_cmd);
             break;
         }
 
@@ -415,10 +456,51 @@ void safe_report_exec_error(int err, const char *actual_cmd, const char *const *
             FLOGF_SAFE(exec, "Out of memory");
             break;
         }
-
+        case EACCES: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': The file could not be accessed.", actual_cmd);
+            break;
+        }
+        case ETXTBSY: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': File is currently open for writing.", actual_cmd);
+            break;
+        }
+        case ELOOP: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': Too many layers of symbolic links. Maybe a loop?", actual_cmd);
+            break;
+        }
+        case EINVAL: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': Unsupported format.", actual_cmd);
+            break;
+        }
+        case EISDIR: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': File is a directory.", actual_cmd);
+            break;
+        }
+        case ENOTDIR: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': A path component is not a directory.", actual_cmd);
+            break;
+        }
+        
+        case EMFILE: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': Too many open files in this process.", actual_cmd);
+            break;
+        }
+        case ENFILE: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': Too many open files on the system.", actual_cmd);
+            break;
+        }
+        case ENAMETOOLONG: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': Name is too long.", actual_cmd);
+            break;
+        }
+        case EPERM: {
+            FLOGF_SAFE(exec, "Failed to execute process '%s': No permission. Either suid/sgid is forbidden or you lack capabilities.", actual_cmd);
+            break;
+        }
         default: {
-            const char *err = safe_strerror(errno);
-            FLOGF_SAFE(exec, "%s", err);
+            char errnum_buff[64];
+            format_long_safe(errnum_buff, err);
+            FLOGF_SAFE(exec, "Failed to execute process '%s', unknown error number %s", actual_cmd, errnum_buff);
             break;
         }
     }

src/wutil.cpp

@@ -203,66 +203,6 @@ int make_fd_blocking(int fd) {
     return err == -1 ? errno : 0;
 }
 
-static inline void safe_append(char *buffer, const char *s, size_t buffsize) {
-    std::strncat(buffer, s, buffsize - std::strlen(buffer) - 1);
-}
-
-// In general, strerror is not async-safe, and therefore we cannot use it directly. So instead we
-// have to grub through sys_nerr and sys_errlist directly On GNU toolchain, this will produce a
-// deprecation warning from the linker (!!), which appears impossible to suppress!
-const char *safe_strerror(int err) {
-#if defined(__UCLIBC__)
-    // uClibc does not have sys_errlist, however, its strerror is believed to be async-safe.
-    // See issue #808.
-    return std::strerror(err);
-#elif defined(HAVE__SYS__ERRS) || defined(HAVE_SYS_ERRLIST)
-#ifdef HAVE_SYS_ERRLIST
-    if (err >= 0 && err < sys_nerr && sys_errlist[err] != nullptr) {
-        return sys_errlist[err];
-    }
-#elif defined(HAVE__SYS__ERRS)
-    extern const char _sys_errs[];
-    extern const int _sys_index[];
-    extern int _sys_num_err;
-
-    if (err >= 0 && err < _sys_num_err) {
-        return &_sys_errs[_sys_index[err]];
-    }
-#endif  // either HAVE__SYS__ERRS or HAVE_SYS_ERRLIST
-#endif  // defined(HAVE__SYS__ERRS) || defined(HAVE_SYS_ERRLIST)
-
-    int saved_err = errno;
-    static char buff[384];  // use a shared buffer for this case
-    char errnum_buff[64];
-    format_long_safe(errnum_buff, err);
-
-    buff[0] = '\0';
-    safe_append(buff, "unknown error (errno was ", sizeof buff);
-    safe_append(buff, errnum_buff, sizeof buff);
-    safe_append(buff, ")", sizeof buff);
-
-    errno = saved_err;
-    return buff;
-}
-
-void safe_perror(const char *message) {
-    // Note we cannot use strerror, because on Linux it uses gettext, which is not safe.
-    int err = errno;
-
-    char buff[384];
-    buff[0] = '\0';
-
-    if (message) {
-        safe_append(buff, message, sizeof buff);
-        safe_append(buff, ": ", sizeof buff);
-    }
-    safe_append(buff, safe_strerror(err), sizeof buff);
-    safe_append(buff, "\n", sizeof buff);
-
-    ignore_result(write(STDERR_FILENO, buff, std::strlen(buff)));
-    errno = err;
-}
-
 /// Wide character realpath. The last path component does not need to be valid. If an error occurs,
 /// wrealpath() returns none() and errno is likely set.
 maybe_t<wcstring> wrealpath(const wcstring &pathname) {

src/wutil.h

@@ -40,12 +40,6 @@ int wunlink(const wcstring &file_name);
 /// Wide character version of perror().
 void wperror(const wchar_t *s);
 
-/// Async-safe version of perror().
-void safe_perror(const char *message);
-
-/// Async-safe version of std::strerror().
-const char *safe_strerror(int err);
-
 /// Wide character version of getcwd().
 wcstring wgetcwd();