xquartz: Replace assert() around side-effecting calls with unconditional error handling

darwin.c and bundle_trampoline.c wrapped calls that allocate memory, add input devices, or spawn a process in
assert(), so building with NDEBUG would silently skip the call and leave the guarded variable uninitialized or
unset while still proceeding as if it had succeeded.

darwin.c now calls FatalError() on failure. bundle_trampoline runs before the server attaches to a terminal or
console, so stderr is not visible there; it now logs via os_log_error() and calls abort().

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

hw/xquartz/darwin.c

@@ -519,9 +519,10 @@ InitInput(int argc, char **argv)
     /* We need to really have rules... or something... */
     XkbSetRulesDflts(&rmlvo);
 
-    assert(Success == AllocDevicePair(serverClient, "xquartz virtual",
-                                      &darwinPointer, &darwinKeyboard,
-                                      DarwinMouseProc, DarwinKeybdProc, FALSE));
+    if (Success != AllocDevicePair(serverClient, "xquartz virtual",
+                                    &darwinPointer, &darwinKeyboard,
+                                    DarwinMouseProc, DarwinKeybdProc, FALSE))
+        FatalError("Failed to allocate xquartz virtual device pair\n");
 
     /* here's the snippet from the current gdk sources:
        if (!strcmp (tmp_name, "pointer"))
@@ -538,15 +539,18 @@ InitInput(int argc, char **argv)
      */
 
     darwinTabletStylus = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletStylus);
+    if (!darwinTabletStylus)
+        FatalError("Failed to add tablet stylus input device\n");
     darwinTabletStylus->name = strdup("pen");
 
     darwinTabletCursor = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletCursor);
+    if (!darwinTabletCursor)
+        FatalError("Failed to add tablet cursor input device\n");
     darwinTabletCursor->name = strdup("cursor");
 
     darwinTabletEraser = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletEraser);
+    if (!darwinTabletEraser)
+        FatalError("Failed to add tablet eraser input device\n");
     darwinTabletEraser->name = strdup("eraser");
 
     DarwinEQInit();
@@ -675,8 +679,10 @@ OsVendorInit(void)
     if (serverGeneration == 1) {
         char *lf;
         char *home = getenv("HOME");
-        assert(home);
-        assert(0 < asprintf(&lf, "%s/Library/Logs/X11", home));
+        if (!home)
+            FatalError("HOME is not set\n");
+        if (asprintf(&lf, "%s/Library/Logs/X11", home) < 0)
+            FatalError("Failed to allocate log directory path\n");
 
         /* Ignore errors.  If EEXIST, we don't care.  If anything else,
          * LogInit will handle it for us.
@@ -684,9 +690,9 @@ OsVendorInit(void)
         (void)mkdir(lf, S_IRWXU | S_IRWXG | S_IRWXO);
         free(lf);
 
-        assert(0 <
-               asprintf(&lf, "%s/Library/Logs/X11/%s.log", home,
-                        bundle_id_prefix));
+        if (asprintf(&lf, "%s/Library/Logs/X11/%s.log", home,
+                     bundle_id_prefix) < 0)
+            FatalError("Failed to allocate log file path\n");
         LogInit(lf, ".old");
         free(lf);
 

hw/xquartz/mach-startup/bundle_trampoline.c

@@ -26,13 +26,15 @@
  * prior written authorization.
  */
 
-#include <assert.h>
 #include <mach-o/dyld.h>
 #include <libgen.h>
+#include <os/log.h>
 #include <spawn.h>
 #include <sys/syslimits.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 
 /* We want XQuartz.app to inherit a login shell environment.  This is handled by the X11.sh
@@ -57,7 +59,10 @@ static char *executable_path() {
     if (_NSGetExecutablePath(buf, &bufsize) == -1) {
         free(buf);
         buf = calloc(1, bufsize);
-        assert(_NSGetExecutablePath(buf, &bufsize) == 0);
+        if (_NSGetExecutablePath(buf, &bufsize) != 0) {
+            os_log_error(OS_LOG_DEFAULT, "Failed to determine executable path");
+            abort();
+        }
     }
 
     return buf;
@@ -72,16 +77,31 @@ int main(int argc, char **argv, char **envp) {
         free(executable);
         asprintf(&executable, "%s/X11", executable_directory);
     }
-    assert(access(executable, X_OK) == 0);
+    if (access(executable, X_OK) != 0) {
+        os_log_error(OS_LOG_DEFAULT, "Cannot execute %{public}s: %{public}s", executable, strerror(errno));
+        abort();
+    }
 
     argv[0] = executable;
 
     posix_spawnattr_t attr;
-    assert(posix_spawnattr_init(&attr) == 0);
-    assert(posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETEXEC) == 0);
+    int error = posix_spawnattr_init(&attr);
+    if (error != 0) {
+        os_log_error(OS_LOG_DEFAULT, "posix_spawnattr_init failed: %{public}s", strerror(error));
+        abort();
+    }
+    error = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETEXEC);
+    if (error != 0) {
+        os_log_error(OS_LOG_DEFAULT, "posix_spawnattr_setflags failed: %{public}s", strerror(error));
+        abort();
+    }
 
     pid_t child_pid;
-    assert(posix_spawn(&child_pid, executable, NULL, &attr, argv, envp) == 0);
+    error = posix_spawn(&child_pid, executable, NULL, &attr, argv, envp);
+    if (error != 0) {
+        os_log_error(OS_LOG_DEFAULT, "posix_spawn failed for %{public}s: %{public}s", executable, strerror(error));
+        abort();
+    }
 
     return EXIT_FAILURE;
 }