Would any Apple user be kind enough to check this trivial X server patch?

Submitted by Adam Richter on May 4, 2019, 2:16 p.m.

Details

Message ID CAGn-TgjCwkbJVeWNKvrnUJ9dkPh+oD2pdL8t3Jy8ROLTOT9oqw@mail.gmail.com
State New
Headers show
Series "Would any Apple user be kind enough to check this trivial X server patch?" ( rev: 2 ) in X.org

Not browsing as part of any series.

Commit Message

Adam Richter May 4, 2019, 2:16 p.m.
Hi, again.

Thanks to Walter Harms for his feedback on my previous proposed patch
to xserver/hw/xquarts/darwin.c, to which Walter requested replacement
of all of the assert statements.  I have attached an updated but
untested patch that attempts to accommodate that request.

Walter is not an Apple user and nobody else has responded to my
request for an Apple user to try my patch.  If anyone wants to try
this patch or provide further feedback, that would be most welcome.
If I don't get any further response in the next couple of day, I
expect I will submit it on the Gitlab server.

Thanks in advance for any further review of this.

Adam

On Thu, May 2, 2019 at 1:02 PM Adam Richter <adamrichter4@gmail.com> wrote:
>
> Hi.
>
> The attached patch fixes three places in hw/xquartz/darwin.c that
> relied on side effects in assert parameters.  That is, they would fail
> if assert() were replaced with a macro that did not evaluate its
> parameters.
>
> I would appreciate it if anyone with an appropriate Apple system would
> try this trivial patch and let me know the results.  Critiques of this
> patch are also welcome, of course.  If I do not get a response to this
> in the next day or two, I expect I will submit the patch on the gitlab
> server, but having someone actually build and run it first would be
> much better.
>
> Thanks in advance for any responses.
>
> Adam

Patch hide | download patch | download mbox

diff --git a/hw/xquartz/darwin.c b/hw/xquartz/darwin.c
index 5c7e96e87..0c60c4bab 100644
--- a/hw/xquartz/darwin.c
+++ b/hw/xquartz/darwin.c
@@ -517,13 +517,19 @@  InitInput(int argc, char **argv)
         .rules   = "base", .model         = "empty", .layout = "empty",
         .variant = NULL,   .options       = NULL
     };
+    int result;
 
     /* We need to really have rules... or something... */
     XkbSetRulesDflts(&rmlvo);
 
-    assert(Success == AllocDevicePair(serverClient, "xquartz virtual",
-                                      &darwinPointer, &darwinKeyboard,
-                                      DarwinMouseProc, DarwinKeybdProc, FALSE));
+    result = AllocDevicePair(serverClient, "xquartz virtual",
+                             &darwinPointer, &darwinKeyboard,
+                             DarwinMouseProc, DarwinKeybdProc, FALSE);
+    if (result != Success) {
+        FatalError("InitInput: AllocDevicePair (...,\"xquartz virtual\",...) "
+                   "returned X windows error %d.\n",
+                   result);
+    }
 
     /* here's the snippet from the current gdk sources:
        if (!strcmp (tmp_name, "pointer"))
@@ -540,15 +546,26 @@  InitInput(int argc, char **argv)
      */
 
     darwinTabletStylus = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletStylus);
+    if (!darwinTabletStylus) {
+        FatalError("InitInput: "
+                   "AddInputDevice(serverClient, DarwinTabletProc, TRUE) "
+                   "failed.\n");
+    }
+
     darwinTabletStylus->name = strdup("pen");
 
     darwinTabletCursor = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletCursor);
+    if(!darwinTabletCursor) {
+        FatalError("InitInput: "
+                   "AddInputDevice(serverClient, DarwinTabletProc, TRUE) "
+                   "failed.\n");
+    }
     darwinTabletCursor->name = strdup("cursor");
 
     darwinTabletEraser = AddInputDevice(serverClient, DarwinTabletProc, TRUE);
-    assert(darwinTabletEraser);
+    if(!darwinTabletEraser) {
+        FatalError("InitInput: AddInputDevice(serverClient, DarwinTabletProc, TRUE) failed.\n");
+    }
     darwinTabletEraser->name = strdup("eraser");
 
     DarwinEQInit();
@@ -677,8 +694,18 @@  OsVendorInit(void)
     if (serverGeneration == 1) {
         char *lf;
         char *home = getenv("HOME");
-        assert(home);
-        assert(0 < asprintf(&lf, "%s/Library/Logs/X11", home));
+        int nbytes;
+
+        if (!home) {
+            FatalError("darwin OsVendorInit: "
+                       "getenv(\"HOME\") returned NULL.\n");
+        }
+
+        nbytes = asprintf(&lf, "%s/Library/Logs/X11", home);
+        if (nbytes <= 0) {
+            FatalError("darwin OsVendorInit: "
+                       "asprintf of log directory name failed.\n");
+        }
 
         /* Ignore errors.  If EEXIST, we don't care.  If anything else,
          * LogInit will handle it for us.
@@ -686,9 +713,12 @@  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));
+        nbytes = asprintf(&lf, "%s/Library/Logs/X11/%s.log", home,
+                          bundle_id_prefix);
+        if (nbytes <= 0) {
+            FatalError("darwin OsVendorInit: "
+                       "asprintf of log file name failed.\n");
+        }
         LogInit(lf, ".old");
         free(lf);