From 8ff12a8e5388c81ec311ac4f53e570896b1343a9 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 ErrorFSigSafe() by ErrorF()" This reverts commit 2d18c353b450da370b9adb53fd96923a26627860. Part-of: --- dix/getevents.c | 10 +++--- dix/ptrveloc.c | 2 +- hw/xfree86/common/xf86Init.c | 10 +++--- include/misc.h | 8 ++--- include/os.h | 4 ++- mi/mieq.c | 18 +++++----- os/backtrace.c | 65 +++++++++++++++++++----------------- os/log.c | 16 +++++++-- os/osinit.c | 6 ++-- 9 files changed, 79 insertions(+), 60 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 5242fd270..4cdd2b711 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1964,8 +1964,8 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, ti = TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin)); if (!ti) { - ErrorF("[dix] %s: unable to %s touch point %u\n", dev->name, - type == XI_TouchBegin ? "begin" : "find", ddx_touchid); + ErrorFSigSafe("[dix] %s: unable to %s touch point %u\n", dev->name, + type == XI_TouchBegin ? "begin" : "find", ddx_touchid); return 0; } client_id = ti->client_id; @@ -1998,14 +1998,16 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, if (!mask_in || !valuator_mask_isset(mask_in, 0) || !valuator_mask_isset(mask_in, 1)) { - ErrorF("%s: Attempted to start touch without x/y (driver bug)\n", dev->name); + ErrorFSigSafe("%s: Attempted to start touch without x/y " + "(driver bug)\n", dev->name); return 0; } break; case XI_TouchUpdate: event->type = ET_TouchUpdate; if (!mask_in || valuator_mask_num_valuators(mask_in) <= 0) { - ErrorF("%s: TouchUpdate with no valuators? Driver bug\n", dev->name); + ErrorFSigSafe("%s: TouchUpdate with no valuators? Driver bug\n", + dev->name); } break; case XI_TouchEnd: diff --git a/dix/ptrveloc.c b/dix/ptrveloc.c index 63fe07b10..192c0f5bb 100644 --- a/dix/ptrveloc.c +++ b/dix/ptrveloc.c @@ -78,7 +78,7 @@ DeletePredictableAccelerationProperties(DeviceIntPtr, /*#define PTRACCEL_DEBUGGING*/ #ifdef PTRACCEL_DEBUGGING -#define DebugAccelF(...) ErrorF("dix/ptraccel: " __VA_ARGS__) +#define DebugAccelF(...) ErrorFSigSafe("dix/ptraccel: " __VA_ARGS__) #else #define DebugAccelF(...) /* */ #endif diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index 0d10794bb..bcfaf8d1d 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -856,12 +856,12 @@ ddxGiveUp(enum ExitCode error) void OsVendorFatalError(const char *f, va_list args) { - ErrorF("\nPlease consult the " XVENDORNAME " support \n\t at " - __VENDORDWEBSUPPORT__ "\n for help. \n"); + ErrorFSigSafe("\nPlease consult the " XVENDORNAME " support \n\t at " + __VENDORDWEBSUPPORT__ "\n for help. \n"); if (xf86LogFile && xf86LogFileWasOpened) - ErrorF("Please also check the log file at \"%s\" for additional " - "information.\n", xf86LogFile); - ErrorF("\n"); + ErrorFSigSafe("Please also check the log file at \"%s\" for additional " + "information.\n", xf86LogFile); + ErrorFSigSafe("\n"); } void diff --git a/include/misc.h b/include/misc.h index 45e4370cc..ba179ff87 100644 --- a/include/misc.h +++ b/include/misc.h @@ -377,10 +377,10 @@ extern _X_EXPORT unsigned long serverGeneration; /* Don't use this directly, use BUG_WARN or BUG_WARN_MSG instead */ #define __BUG_WARN_MSG(cond, with_msg, ...) \ do { if (cond) { \ - ErrorF("BUG: triggered 'if (" #cond ")'\n"); \ - ErrorF("BUG: %s:%u in %s()\n", \ - __FILE__, __LINE__, __func__); \ - if (with_msg) ErrorF(__VA_ARGS__); \ + ErrorFSigSafe("BUG: triggered 'if (" #cond ")'\n"); \ + ErrorFSigSafe("BUG: %s:%u in %s()\n", \ + __FILE__, __LINE__, __func__); \ + if (with_msg) ErrorFSigSafe(__VA_ARGS__); \ xorg_backtrace(); \ } } while(0) diff --git a/include/os.h b/include/os.h index 67b1cfbb3..15e6e10e7 100644 --- a/include/os.h +++ b/include/os.h @@ -359,6 +359,9 @@ extern _X_EXPORT void VErrorFSigSafe(const char *f, va_list args) _X_ATTRIBUTE_PRINTF(1, 0); extern _X_EXPORT void +ErrorFSigSafe(const char *f, ...) +_X_ATTRIBUTE_PRINTF(1, 2); +extern _X_EXPORT void LogPrintMarkers(void); extern _X_EXPORT void @@ -373,7 +376,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__) -#define ErrorFSigSafe(...) ErrorF(__VA_ARGS__) /* only for backwards compat with drivers that haven't kept up yet (xf86-video-intel) diff --git a/mi/mieq.c b/mi/mieq.c index 6d9e3eb87..852732b98 100644 --- a/mi/mieq.c +++ b/mi/mieq.c @@ -227,22 +227,22 @@ mieqEnqueue(DeviceIntPtr pDev, InternalEvent *e) */ miEventQueue.dropped++; if (miEventQueue.dropped == 1) { - ErrorF("[mi] EQ overflowing. Additional events will be " - "discarded until existing events are processed.\n"); + ErrorFSigSafe("[mi] EQ overflowing. Additional events will be " + "discarded until existing events are processed.\n"); xorg_backtrace(); - ErrorF("[mi] These backtraces from mieqEnqueue may point to " - "a culprit higher up the stack.\n"); - ErrorF("[mi] mieq is *NOT* the cause. It is a victim.\n"); + ErrorFSigSafe("[mi] These backtraces from mieqEnqueue may point to " + "a culprit higher up the stack.\n"); + ErrorFSigSafe("[mi] mieq is *NOT* the cause. It is a victim.\n"); } else if (miEventQueue.dropped % QUEUE_DROP_BACKTRACE_FREQUENCY == 0 && miEventQueue.dropped / QUEUE_DROP_BACKTRACE_FREQUENCY <= QUEUE_DROP_BACKTRACE_MAX) { - ErrorF("[mi] EQ overflow continuing. %zu events have been " - "dropped.\n", miEventQueue.dropped); + ErrorFSigSafe("[mi] EQ overflow continuing. %zu events have been " + "dropped.\n", miEventQueue.dropped); if (miEventQueue.dropped / QUEUE_DROP_BACKTRACE_FREQUENCY == QUEUE_DROP_BACKTRACE_MAX) { - ErrorF("[mi] No further overflow reports will be " - "reported until the clog is cleared.\n"); + ErrorFSigSafe("[mi] No further overflow reports will be " + "reported until the clog is cleared.\n"); } xorg_backtrace(); } diff --git a/os/backtrace.c b/os/backtrace.c index aaa27d29e..281747de4 100644 --- a/os/backtrace.c +++ b/os/backtrace.c @@ -82,21 +82,21 @@ print_registers(int frame, unw_cursor_t cursor) frame++; ret = unw_step(&cursor); if (ret < 0) { - ErrorF("unw_step failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_step failed: %s [%d]\n", unw_strerror(ret), ret); return; } - ErrorF("\n"); - ErrorF("Registers at frame #%d:\n", frame); + ErrorFSigSafe("\n"); + ErrorFSigSafe("Registers at frame #%d:\n", frame); for (i = 0; i < num_regs; i++) { unw_word_t val; ret = unw_get_reg(&cursor, regs[i].regnum, &val); if (ret < 0) { - ErrorF("unw_get_reg(%s) failed: %s [%d]\n", + ErrorFSigSafe("unw_get_reg(%s) failed: %s [%d]\n", regs[i].name, unw_strerror(ret), ret); } else { - ErrorF(" %s: 0x%" PRIxPTR "\n", regs[i].name, val); + ErrorFSigSafe(" %s: 0x%" PRIxPTR "\n", regs[i].name, val); } } } @@ -117,23 +117,26 @@ xorg_backtrace(void) pip.unwind_info = NULL; ret = unw_getcontext(&context); if (ret) { - ErrorF("unw_getcontext failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_getcontext failed: %s [%d]\n", unw_strerror(ret), + ret); return; } ret = unw_init_local(&cursor, &context); if (ret) { - ErrorF("unw_init_local failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_init_local failed: %s [%d]\n", unw_strerror(ret), + ret); return; } - ErrorF("\n"); - ErrorF("Backtrace:\n"); + ErrorFSigSafe("\n"); + ErrorFSigSafe("Backtrace:\n"); ret = unw_step(&cursor); while (ret > 0) { ret = unw_get_proc_info(&cursor, &pip); if (ret) { - ErrorF("unw_get_proc_info failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_get_proc_info failed: %s [%d]\n", + unw_strerror(ret), ret); break; } @@ -141,7 +144,8 @@ xorg_backtrace(void) ret = unw_get_proc_name(&cursor, procname, 256, &off); if (ret && ret != -UNW_ENOMEM) { if (ret != -UNW_EUNSPEC) - ErrorF("unw_get_proc_name failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_get_proc_name failed: %s [%d]\n", + unw_strerror(ret), ret); procname[0] = '?'; procname[1] = 0; } @@ -159,22 +163,22 @@ xorg_backtrace(void) signal_cursor = cursor; signal_frame = i; - ErrorF("%u: \n", i++); + ErrorFSigSafe("%u: \n", i++); } else { - ErrorF("%u: %s (%s%s+0x%x) [%p]\n", i++, filename, procname, - ret == -UNW_ENOMEM ? "..." : "", (int)off, + ErrorFSigSafe("%u: %s (%s%s+0x%x) [%p]\n", i++, filename, procname, + ret == -UNW_ENOMEM ? "..." : "", (int)off, (void *)(uintptr_t)(ip)); } ret = unw_step(&cursor); if (ret < 0) - ErrorF("unw_step failed: %s [%d]\n", unw_strerror(ret), ret); + ErrorFSigSafe("unw_step failed: %s [%d]\n", unw_strerror(ret), ret); } if (signal_frame >= 0) print_registers(signal_frame, signal_cursor); - ErrorF("\n"); + ErrorFSigSafe("\n"); } #else /* HAVE_LIBUNWIND */ #ifdef HAVE_BACKTRACE @@ -193,19 +197,19 @@ xorg_backtrace(void) int size, i; Dl_info info; - ErrorF("\n"); - ErrorF("Backtrace:\n"); + ErrorFSigSafe("\n"); + ErrorFSigSafe("Backtrace:\n"); size = backtrace(array, BT_SIZE); for (i = 0; i < size; i++) { int rc = dladdr(array[i], &info); if (rc == 0) { - ErrorF("%u: ?? [%p]\n", i, array[i]); + ErrorFSigSafe("%u: ?? [%p]\n", i, array[i]); continue; } mod = (info.dli_fname && *info.dli_fname) ? info.dli_fname : "(vdso)"; if (info.dli_saddr) - ErrorF( + ErrorFSigSafe( "%u: %s (%s+0x%x) [%p]\n", i, mod, @@ -214,7 +218,7 @@ xorg_backtrace(void) (char *) info.dli_saddr), array[i]); else - ErrorF( + ErrorFSigSafe( "%u: %s (%p+0x%x) [%p]\n", i, mod, @@ -223,7 +227,7 @@ xorg_backtrace(void) (char *) info.dli_fbase), array[i]); } - ErrorF("\n"); + ErrorFSigSafe("\n"); } #else /* not glibc or glibc < 2.1 */ @@ -261,7 +265,7 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg) strcpy(signame, "unknown"); } - ErrorF("** Signal %u (%s)\n", signo, signame); + ErrorFSigSafe("** Signal %u (%s)\n", signo, signame); } snprintf(header, sizeof(header), "%d: 0x%lx", depth, pc); @@ -279,7 +283,8 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg) symname = "
"; offset = pc - (uintptr_t) dlinfo.dli_fbase; } - ErrorF("%s: %s:%s+0x%x\n", header, dlinfo.dli_fname, symname, offset); + ErrorFSigSafe("%s: %s:%s+0x%x\n", header, dlinfo.dli_fname, symname, + offset); } else { @@ -287,7 +292,7 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg) * probably poke elfloader here, but haven't written that code yet, * so we just print the pc. */ - ErrorF("%s\n", header); + ErrorFSigSafe("%s\n", header); } return 0; @@ -341,7 +346,7 @@ xorg_backtrace_pstack(void) if (bytesread > 0) { btline[bytesread] = 0; - ErrorF("%s", btline); + ErrorFSigSafe("%s", btline); } else if ((bytesread < 0) || ((errno != EINTR) && (errno != EAGAIN))) done = 1; @@ -361,8 +366,8 @@ void xorg_backtrace(void) { - ErrorF("\n"); - ErrorF("Backtrace:\n"); + ErrorFSigSafe("\n"); + ErrorFSigSafe("Backtrace:\n"); #ifdef HAVE_PSTACK /* First try fork/exec of pstack - otherwise fall back to walkcontext @@ -379,9 +384,9 @@ xorg_backtrace(void) walkcontext(&u, xorg_backtrace_frame, &depth); else #endif - ErrorF("Failed to get backtrace info: %s\n", strerror(errno)); + ErrorFSigSafe("Failed to get backtrace info: %s\n", strerror(errno)); } - ErrorF("\n"); + ErrorFSigSafe("\n"); } #else diff --git a/os/log.c b/os/log.c index 58a39c5c0..46747e48b 100644 --- a/os/log.c +++ b/os/log.c @@ -856,9 +856,9 @@ FatalError(const char *f, ...) static Bool beenhere = FALSE; if (beenhere) - ErrorF("\nFatalError re-entered, aborting\n"); + ErrorFSigSafe("\nFatalError re-entered, aborting\n"); else - ErrorF("\nFatal server error:\n"); + ErrorFSigSafe("\nFatal server error:\n"); va_start(args, f); @@ -877,7 +877,7 @@ FatalError(const char *f, ...) #endif VErrorFSigSafe(f, args); va_end(args); - ErrorF("\n"); + ErrorFSigSafe("\n"); if (!beenhere) OsVendorFatalError(f, args2); va_end(args2); @@ -914,6 +914,16 @@ VErrorFSigSafe(const char *f, va_list args) LogVMessageVerb(X_ERROR, -1, f, args); } +void +ErrorFSigSafe(const char *f, ...) +{ + va_list args; + + va_start(args, f); + VErrorFSigSafe(f, args); + va_end(args); +} + void LogPrintMarkers(void) { diff --git a/os/osinit.c b/os/osinit.c index 3e18b51b0..61dfbd2c2 100644 --- a/os/osinit.c +++ b/os/osinit.c @@ -136,8 +136,8 @@ OsSigHandler(int signo) #ifdef SA_SIGINFO if (sip->si_code == SI_USER) { - ErrorF("Received signal %u sent by process %u, uid %u\n", signo, - sip->si_pid, sip->si_uid); + ErrorFSigSafe("Received signal %u sent by process %u, uid %u\n", signo, + sip->si_pid, sip->si_uid); } else { switch (signo) { @@ -145,7 +145,7 @@ OsSigHandler(int signo) case SIGBUS: case SIGILL: case SIGFPE: - ErrorF("%s at address %p\n", strsignal(signo), sip->si_addr); + ErrorFSigSafe("%s at address %p\n", strsignal(signo), sip->si_addr); } } #endif