From 64a4a87c12daaaecb9aa3c8717dd1d038afb7903 Mon Sep 17 00:00:00 2001 From: Adam Richter Date: Fri, 10 May 2019 09:44:12 -0700 Subject: [PATCH 1/4] xquartz: Move side effects out of assert statements in hx/xquartz/darwin.c. Thanks to Walter Harms for pointing out that the previous code would have failed if it had been compiled with NDEBUG. --- hw/xquartz/darwin.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/hw/xquartz/darwin.c b/hw/xquartz/darwin.c index 5c7e96e87..1026bcf02 100644 --- a/hw/xquartz/darwin.c +++ b/hw/xquartz/darwin.c @@ -517,13 +517,15 @@ 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); + assert(result == Success); /* here's the snippet from the current gdk sources: if (!strcmp (tmp_name, "pointer")) @@ -677,8 +679,12 @@ OsVendorInit(void) if (serverGeneration == 1) { char *lf; char *home = getenv("HOME"); + int nbytes; + assert(home); - assert(0 < asprintf(&lf, "%s/Library/Logs/X11", home)); + + nbytes = asprintf(&lf, "%s/Library/Logs/X11", home); + assert(nbytes > 0); /* Ignore errors. If EEXIST, we don't care. If anything else, * LogInit will handle it for us. @@ -686,9 +692,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)); + nbytes = asprintf(&lf, "%s/Library/Logs/X11/%s.log", home, + bundle_id_prefix); + assert(nbytes > 0); LogInit(lf, ".old"); free(lf); From 47cdc1933a475c34e8cbe51395f992d2c4696608 Mon Sep 17 00:00:00 2001 From: Adam Richter Date: Fri, 10 May 2019 09:51:58 -0700 Subject: [PATCH 2/4] xquartz: Replace assert with FatalError in hw/xquartz/darwin.c. Thanks to Walter Harms and Matthieu Herrb for recommending replacement of assert statements in places that are not truly internal inconsistencies with proper error handling when possible. --- hw/xquartz/darwin.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/hw/xquartz/darwin.c b/hw/xquartz/darwin.c index 1026bcf02..0c60c4bab 100644 --- a/hw/xquartz/darwin.c +++ b/hw/xquartz/darwin.c @@ -525,7 +525,11 @@ InitInput(int argc, char **argv) result = AllocDevicePair(serverClient, "xquartz virtual", &darwinPointer, &darwinKeyboard, DarwinMouseProc, DarwinKeybdProc, FALSE); - assert(result == Success); + 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")) @@ -542,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(); @@ -681,10 +696,16 @@ OsVendorInit(void) char *home = getenv("HOME"); int nbytes; - assert(home); + if (!home) { + FatalError("darwin OsVendorInit: " + "getenv(\"HOME\") returned NULL.\n"); + } nbytes = asprintf(&lf, "%s/Library/Logs/X11", home); - assert(nbytes > 0); + 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. @@ -694,7 +715,10 @@ OsVendorInit(void) nbytes = asprintf(&lf, "%s/Library/Logs/X11/%s.log", home, bundle_id_prefix); - assert(nbytes > 0); + if (nbytes <= 0) { + FatalError("darwin OsVendorInit: " + "asprintf of log file name failed.\n"); + } LogInit(lf, ".old"); free(lf); From c033b0be2b1ad4c49dbd1316c93ff0f7fab583b2 Mon Sep 17 00:00:00 2001 From: Adam Richter Date: Fri, 10 May 2019 09:44:12 -0700 Subject: [PATCH 3/4] xquartz: Move side effects out of assert statements in hx/xquartz/darwin.c. Thanks to Walter Harms for pointing out that the previous code would have failed if it had been compiled with NDEBUG. --- hw/xquartz/darwin.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/hw/xquartz/darwin.c b/hw/xquartz/darwin.c index 5c7e96e87..1026bcf02 100644 --- a/hw/xquartz/darwin.c +++ b/hw/xquartz/darwin.c @@ -517,13 +517,15 @@ 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); + assert(result == Success); /* here's the snippet from the current gdk sources: if (!strcmp (tmp_name, "pointer")) @@ -677,8 +679,12 @@ OsVendorInit(void) if (serverGeneration == 1) { char *lf; char *home = getenv("HOME"); + int nbytes; + assert(home); - assert(0 < asprintf(&lf, "%s/Library/Logs/X11", home)); + + nbytes = asprintf(&lf, "%s/Library/Logs/X11", home); + assert(nbytes > 0); /* Ignore errors. If EEXIST, we don't care. If anything else, * LogInit will handle it for us. @@ -686,9 +692,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)); + nbytes = asprintf(&lf, "%s/Library/Logs/X11/%s.log", home, + bundle_id_prefix); + assert(nbytes > 0); LogInit(lf, ".old"); free(lf); From 6c361db8830b9b62cf09084c4d0b7a3e51195c83 Mon Sep 17 00:00:00 2001 From: Adam Richter Date: Fri, 10 May 2019 09:51:58 -0700 Subject: [PATCH 4/4] xquartz: Replace assert with FatalError in hw/xquartz/darwin.c. Thanks to Walter Harms and Matthieu Herrb for recommending replacement of assert statements in places that are not truly internal inconsistencies with proper error handling when possible. --- hw/xquartz/darwin.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/hw/xquartz/darwin.c b/hw/xquartz/darwin.c index 1026bcf02..0c60c4bab 100644 --- a/hw/xquartz/darwin.c +++ b/hw/xquartz/darwin.c @@ -525,7 +525,11 @@ InitInput(int argc, char **argv) result = AllocDevicePair(serverClient, "xquartz virtual", &darwinPointer, &darwinKeyboard, DarwinMouseProc, DarwinKeybdProc, FALSE); - assert(result == Success); + 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")) @@ -542,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(); @@ -681,10 +696,16 @@ OsVendorInit(void) char *home = getenv("HOME"); int nbytes; - assert(home); + if (!home) { + FatalError("darwin OsVendorInit: " + "getenv(\"HOME\") returned NULL.\n"); + } nbytes = asprintf(&lf, "%s/Library/Logs/X11", home); - assert(nbytes > 0); + 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. @@ -694,7 +715,10 @@ OsVendorInit(void) nbytes = asprintf(&lf, "%s/Library/Logs/X11/%s.log", home, bundle_id_prefix); - assert(nbytes > 0); + if (nbytes <= 0) { + FatalError("darwin OsVendorInit: " + "asprintf of log file name failed.\n"); + } LogInit(lf, ".old"); free(lf);