From 6a9b2f37bb6f9ee760f12edd066c12441a2fb1f7 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Mon, 23 Jun 2025 16:25:31 -0700 Subject: [PATCH] Revert "os: log: replace LogMessageVerbSigSafe() by LogMessageVerb()" This reverts commit dd37cc4855f4b53aa35059dadac85a2b3173e796. Part-of: --- glx/glxdri2.c | 2 +- .../drivers/inputtest/xf86-input-inputtest.c | 6 +- hw/xfree86/loader/loader.c | 2 +- hw/xfree86/loader/loadmod.c | 4 +- include/os.h | 4 +- os/log.c | 14 ++++- os/osinit.c | 6 +- test/signal-logging.c | 62 +++++++++---------- 8 files changed, 57 insertions(+), 43 deletions(-) diff --git a/glx/glxdri2.c b/glx/glxdri2.c index 4c5bc8446..c4e02b7b7 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -805,7 +805,7 @@ glxDRILeaveVT(ScrnInfoPtr scrn) __GLXDRIscreen *screen = (__GLXDRIscreen *) glxGetScreen(xf86ScrnToScreen(scrn)); - LogMessageVerb(X_INFO, -1, "AIGLX: Suspending AIGLX clients for VT switch\n"); + LogMessageVerbSigSafe(X_INFO, -1, "AIGLX: Suspending AIGLX clients for VT switch\n"); glxSuspendClients(); diff --git a/hw/xfree86/drivers/inputtest/xf86-input-inputtest.c b/hw/xfree86/drivers/inputtest/xf86-input-inputtest.c index 52e58f13b..294e35747 100644 --- a/hw/xfree86/drivers/inputtest/xf86-input-inputtest.c +++ b/hw/xfree86/drivers/inputtest/xf86-input-inputtest.c @@ -126,9 +126,9 @@ notify_sync_finished(ClientPtr ptr, void *closure) already shut down and the descriptor is closed. */ if (write(fd, &response, response.header.length) != response.header.length) { - LogMessageVerb(X_ERROR, 0, - "inputtest: Failed to write sync response: %s\n", - strerror(errno)); + LogMessageVerbSigSafe(X_ERROR, 0, + "inputtest: Failed to write sync response: %s\n", + strerror(errno)); } input_unlock(); return TRUE; diff --git a/hw/xfree86/loader/loader.c b/hw/xfree86/loader/loader.c index 5af7a54f3..4696b639d 100644 --- a/hw/xfree86/loader/loader.c +++ b/hw/xfree86/loader/loader.c @@ -141,7 +141,7 @@ LoaderSymbolFromModule(void *handle, const char *name) void LoaderUnload(const char *name, void *handle) { - LogMessageVerb(X_INFO, 1, "Unloading %s\n", name); + LogMessageVerbSigSafe(X_INFO, 1, "Unloading %s\n", name); if (handle) dlclose(handle); } diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c index 9bb7aa47b..cca11028c 100644 --- a/hw/xfree86/loader/loadmod.c +++ b/hw/xfree86/loader/loadmod.c @@ -854,9 +854,9 @@ UnloadModule(void *_mod) const char *name = mod->VersionInfo->modname; if (mod->parent) - LogMessageVerb(X_INFO, 3, "UnloadSubModule: \"%s\"\n", name); + LogMessageVerbSigSafe(X_INFO, 3, "UnloadSubModule: \"%s\"\n", name); else - LogMessageVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", name); + LogMessageVerbSigSafe(X_INFO, 3, "UnloadModule: \"%s\"\n", name); if (mod->TearDownData != ModuleDuplicated) { if ((mod->TearDownProc) && (mod->TearDownData)) diff --git a/include/os.h b/include/os.h index 15e6e10e7..2f43e4eda 100644 --- a/include/os.h +++ b/include/os.h @@ -324,6 +324,9 @@ _X_ATTRIBUTE_PRINTF(3, 4); extern _X_EXPORT void LogMessage(MessageType type, const char *format, ...) _X_ATTRIBUTE_PRINTF(2, 3); +extern _X_EXPORT void +LogMessageVerbSigSafe(MessageType type, int verb, const char *format, ...) +_X_ATTRIBUTE_PRINTF(3, 4); extern _X_EXPORT void LogVHdrMessageVerb(MessageType type, int verb, @@ -375,7 +378,6 @@ typedef _sigset_t sigset_t; /* should not be used anymore, just for backwards compat with drivers */ #define LogVMessageVerbSigSafe(...) LogVMessageVerb(__VA_ARGS__) -#define LogMessageVerbSigSafe(...) LogMessageVerb(__VA_ARGS__) /* only for backwards compat with drivers that haven't kept up yet (xf86-video-intel) diff --git a/os/log.c b/os/log.c index 46747e48b..c901234fe 100644 --- a/os/log.c +++ b/os/log.c @@ -302,7 +302,7 @@ LogClose(enum ExitCode error) { if (logFile) { int msgtype = (error == EXIT_NO_ERROR) ? X_INFO : X_ERROR; - LogMessageVerb(msgtype, -1, + LogMessageVerbSigSafe(msgtype, -1, "Server terminated %s (%d). Closing log file.\n", (error == EXIT_NO_ERROR) ? "successfully" : "with error", error); @@ -695,7 +695,7 @@ LogVMessageVerb(MessageType type, int verb, const char *format, va_list args) writeLog(verb, buf, len); } -/* Log message with verbosity level specified. -- signal safe */ +/* Log message with verbosity level specified. */ void LogMessageVerb(MessageType type, int verb, const char *format, ...) { @@ -717,6 +717,16 @@ LogMessage(MessageType type, const char *format, ...) va_end(ap); } +/* Log a message using only signal safe functions. */ +void +LogMessageVerbSigSafe(MessageType type, int verb, const char *format, ...) +{ + va_list ap; + va_start(ap, format); + LogVMessageVerb(type, verb, format, ap); + va_end(ap); +} + void LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format, va_list msg_args, const char *hdr_format, va_list hdr_args) diff --git a/os/osinit.c b/os/osinit.c index 61dfbd2c2..3a132faee 100644 --- a/os/osinit.c +++ b/os/osinit.c @@ -119,8 +119,10 @@ OsSigHandler(int signo) if (signo == SIGNAL_FOR_RTLD_ERROR) { const char *dlerr = dlerror(); - if (dlerr) - LogMessageVerb(X_ERROR, 1, "Dynamic loader error: %s\n", dlerr); + if (dlerr) { + LogMessageVerbSigSafe(X_ERROR, 1, + "Dynamic loader error: %s\n", dlerr); + } } #endif /* RTLD_DI_SETSIGNAL */ diff --git a/test/signal-logging.c b/test/signal-logging.c index 22a01631c..db8efa852 100644 --- a/test/signal-logging.c +++ b/test/signal-logging.c @@ -202,59 +202,59 @@ static void logging_format(void) } while (0) /* boring test message */ - LogMessageVerb(X_ERROR, 1, "test message\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "test message\n"); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) test message\n") == 0); /* long buf is truncated to "....en\n" */ - LogMessageVerb(X_ERROR, 1, buf); + LogMessageVerbSigSafe(X_ERROR, 1, buf); read_log_msg(logmsg); assert(strcmp(&logmsg[strlen(logmsg) - 3], "en\n") == 0); /* same thing, this time as string substitution */ - LogMessageVerb(X_ERROR, 1, "%s", buf); + LogMessageVerbSigSafe(X_ERROR, 1, "%s", buf); read_log_msg(logmsg); assert(strcmp(&logmsg[strlen(logmsg) - 3], "en\n") == 0); /* strings containing placeholders should just work */ - LogMessageVerb(X_ERROR, 1, "%s\n", str); + LogMessageVerbSigSafe(X_ERROR, 1, "%s\n", str); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) %s %d %u %% %p %i\n") == 0); /* literal % */ - LogMessageVerb(X_ERROR, 1, "test %%\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "test %%\n"); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) test %\n") == 0); /* character */ - LogMessageVerb(X_ERROR, 1, "test %c\n", 'a'); + LogMessageVerbSigSafe(X_ERROR, 1, "test %c\n", 'a'); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) test a\n") == 0); /* something unsupported % */ - LogMessageVerb(X_ERROR, 1, "test %Q\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "test %Q\n"); read_log_msg(logmsg); assert(strstr(logmsg, "BUG") != NULL); - LogMessageVerb(X_ERROR, 1, "\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "\n"); fseek(f, 0, SEEK_END); /* string substitution */ - LogMessageVerb(X_ERROR, 1, "%s\n", "substituted string"); + LogMessageVerbSigSafe(X_ERROR, 1, "%s\n", "substituted string"); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) substituted string\n") == 0); /* Invalid format */ - LogMessageVerb(X_ERROR, 1, "%4", 4); + LogMessageVerbSigSafe(X_ERROR, 1, "%4", 4); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) ") == 0); - LogMessageVerb(X_ERROR, 1, "\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "\n"); fseek(f, 0, SEEK_END); /* %hld is bogus */ - LogMessageVerb(X_ERROR, 1, "%hld\n", 4); + LogMessageVerbSigSafe(X_ERROR, 1, "%hld\n", 4); read_log_msg(logmsg); assert(strstr(logmsg, "BUG") != NULL); - LogMessageVerb(X_ERROR, 1, "\n"); + LogMessageVerbSigSafe(X_ERROR, 1, "\n"); fseek(f, 0, SEEK_END); /* number substitution */ @@ -262,12 +262,12 @@ static void logging_format(void) do { char expected[30]; sprintf(expected, "(EE) %u\n", ui); - LogMessageVerb(X_ERROR, 1, "%u\n", ui); + LogMessageVerbSigSafe(X_ERROR, 1, "%u\n", ui); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %x\n", ui); - LogMessageVerb(X_ERROR, 1, "%x\n", ui); + LogMessageVerbSigSafe(X_ERROR, 1, "%x\n", ui); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); @@ -281,21 +281,21 @@ static void logging_format(void) do { char expected[30]; sprintf(expected, "(EE) %lu\n", lui); - LogMessageVerb(X_ERROR, 1, "%lu\n", lui); + LogMessageVerbSigSafe(X_ERROR, 1, "%lu\n", lui); read_log_msg(logmsg); sprintf(expected, "(EE) %lld\n", (unsigned long long)ui); - LogMessageVerb(X_ERROR, 1, "%lld\n", (unsigned long long)ui); + LogMessageVerbSigSafe(X_ERROR, 1, "%lld\n", (unsigned long long)ui); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %lx\n", lui); - LogMessageVerb(X_ERROR, 1, "%lx\n", lui); + LogMessageVerbSigSafe(X_ERROR, 1, "%lx\n", lui); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %llx\n", (unsigned long long)ui); - LogMessageVerb(X_ERROR, 1, "%llx\n", (unsigned long long)ui); + LogMessageVerbSigSafe(X_ERROR, 1, "%llx\n", (unsigned long long)ui); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); @@ -310,12 +310,12 @@ static void logging_format(void) do { char expected[30]; sprintf(expected, "(EE) %d\n", i); - LogMessageVerb(X_ERROR, 1, "%d\n", i); + LogMessageVerbSigSafe(X_ERROR, 1, "%d\n", i); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %d\n", i | INT_MIN); - LogMessageVerb(X_ERROR, 1, "%d\n", i | INT_MIN); + LogMessageVerbSigSafe(X_ERROR, 1, "%d\n", i | INT_MIN); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); @@ -329,22 +329,22 @@ static void logging_format(void) do { char expected[30]; sprintf(expected, "(EE) %ld\n", li); - LogMessageVerb(X_ERROR, 1, "%ld\n", li); + LogMessageVerbSigSafe(X_ERROR, 1, "%ld\n", li); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %ld\n", li | LONG_MIN); - LogMessageVerb(X_ERROR, 1, "%ld\n", li | LONG_MIN); + LogMessageVerbSigSafe(X_ERROR, 1, "%ld\n", li | LONG_MIN); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %lld\n", (long long)li); - LogMessageVerb(X_ERROR, 1, "%lld\n", (long long)li); + LogMessageVerbSigSafe(X_ERROR, 1, "%lld\n", (long long)li); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); sprintf(expected, "(EE) %lld\n", (long long)(li | LONG_MIN)); - LogMessageVerb(X_ERROR, 1, "%lld\n", (long long)(li | LONG_MIN)); + LogMessageVerbSigSafe(X_ERROR, 1, "%lld\n", (long long)(li | LONG_MIN)); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); @@ -357,7 +357,7 @@ static void logging_format(void) /* pointer substitution */ /* we print a null-pointer differently to printf */ - LogMessageVerb(X_ERROR, 1, "%p\n", NULL); + LogMessageVerbSigSafe(X_ERROR, 1, "%p\n", NULL); read_log_msg(logmsg); assert(strcmp(logmsg, "(EE) 0x0\n") == 0); @@ -369,7 +369,7 @@ static void logging_format(void) #else sprintf(expected, "(EE) %p\n", (void*)ptr); #endif - LogMessageVerb(X_ERROR, 1, "%p\n", (void*)ptr); + LogMessageVerbSigSafe(X_ERROR, 1, "%p\n", (void*)ptr); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); ptr <<= 1; @@ -380,20 +380,20 @@ static void logging_format(void) double d = float_tests[i]; char expected[30]; sprintf(expected, "(EE) %.2f\n", d); - LogMessageVerb(X_ERROR, 1, "%f\n", d); + LogMessageVerbSigSafe(X_ERROR, 1, "%f\n", d); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); /* test for length modifiers, we just ignore them atm */ - LogMessageVerb(X_ERROR, 1, "%.3f\n", d); + LogMessageVerbSigSafe(X_ERROR, 1, "%.3f\n", d); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); - LogMessageVerb(X_ERROR, 1, "%3f\n", d); + LogMessageVerbSigSafe(X_ERROR, 1, "%3f\n", d); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); - LogMessageVerb(X_ERROR, 1, "%.0f\n", d); + LogMessageVerbSigSafe(X_ERROR, 1, "%.0f\n", d); read_log_msg(logmsg); assert(strcmp(logmsg, expected) == 0); }