diff options
| -rw-r--r-- | dix/getevents.c | 18 | ||||
| -rw-r--r-- | include/misc.h | 1 | ||||
| -rw-r--r-- | mi/mipointer.c | 2 | ||||
| -rw-r--r-- | os/log.c | 50 | ||||
| -rw-r--r-- | os/utils.c | 14 | ||||
| -rw-r--r-- | test/signal-logging.c | 210 |
6 files changed, 270 insertions, 25 deletions
diff --git a/dix/getevents.c b/dix/getevents.c index b3bb162ae..4e62507aa 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1327,6 +1327,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, RawDeviceEvent *raw; double screenx = 0.0, screeny = 0.0; /* desktop coordinate system */ double devx = 0.0, devy = 0.0; /* desktop-wide in device coords */ + int sx, sy; /* for POINTER_SCREEN */ ValuatorMask mask; ScreenPtr scr; @@ -1369,8 +1370,11 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, /* valuators are in driver-native format (rel or abs) */ if (flags & POINTER_ABSOLUTE) { - if (flags & POINTER_SCREEN) /* valuators are in screen coords */ + if (flags & POINTER_SCREEN) { /* valuators are in screen coords */ + sx = valuator_mask_get(&mask, 0); + sy = valuator_mask_get(&mask, 1); scale_from_screen(pDev, &mask); + } transformAbsolute(pDev, &mask); clipAbsolute(pDev, &mask); @@ -1388,6 +1392,18 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, /* valuators are in device coordinate system in absolute coordinates */ scale_to_desktop(pDev, &mask, &devx, &devy, &screenx, &screeny); + + /* #53037 XWarpPointer's scaling back and forth between screen and + device may leave us with rounding errors. End result is that the + pointer doesn't end up on the pixel it should. + Avoid this by forcing screenx/screeny back to what the input + coordinates were. + */ + if (flags & POINTER_SCREEN) { + screenx = sx; + screeny = sy; + } + scr = positionSprite(pDev, (flags & POINTER_ABSOLUTE) ? Absolute : Relative, &mask, &devx, &devy, &screenx, &screeny); diff --git a/include/misc.h b/include/misc.h index 7f7f221a8..348717676 100644 --- a/include/misc.h +++ b/include/misc.h @@ -247,6 +247,7 @@ padding_for_int32(const int bytes) extern char **xstrtokenize(const char *str, const char *separators); +extern void FormatInt64(int64_t num, char *string); extern void FormatUInt64(uint64_t num, char *string); extern void FormatUInt64Hex(uint64_t num, char *string); diff --git a/mi/mipointer.c b/mi/mipointer.c index a56838ead..4defaf5ec 100644 --- a/mi/mipointer.c +++ b/mi/mipointer.c @@ -575,7 +575,7 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, miPointerPtr pPointer; - if (!pDev || !pDev->coreEvents) + if (!pDev) return NULL; pPointer = MIPOINTER(pDev); @@ -290,6 +290,7 @@ pnprintf(char *string, size_t size, const char *f, va_list args) int p_len; int i; uint64_t ui; + int64_t si; for (; f_idx < f_len && s_idx < size - 1; f_idx++) { if (f[f_idx] != '%') { @@ -314,6 +315,15 @@ pnprintf(char *string, size_t size, const char *f, va_list args) for (i = 0; i < p_len && s_idx < size - 1; i++) string[s_idx++] = number[i]; break; + case 'i': + case 'd': + si = va_arg(args, int); + FormatInt64(si, number); + p_len = strlen_sigsafe(number); + + for (i = 0; i < p_len && s_idx < size - 1; i++) + string[s_idx++] = number[i]; + break; case 'p': string[s_idx++] = '0'; @@ -364,7 +374,7 @@ LogSWrite(int verb, const char *buf, size_t len, Bool end_line) if (verb < 0 || logFileVerbosity >= verb) { if (inSignalContext && logFileFd >= 0) { write(logFileFd, buf, len); -#ifdef WIN32 +#ifndef WIN32 if (logFlush && logSync) fsync(logFileFd); #endif @@ -457,6 +467,7 @@ LogMessageTypeVerbString(MessageType type, int verb) void LogVMessageVerb(MessageType type, int verb, const char *format, va_list args) { + static unsigned int warned; const char *type_str; char buf[1024]; const size_t size = sizeof(buf); @@ -464,13 +475,17 @@ LogVMessageVerb(MessageType type, int verb, const char *format, va_list args) size_t len = 0; if (inSignalContext) { - BUG_WARN_MSG(inSignalContext, - "Warning: attempting to log data in a signal unsafe " - "manner while in signal context. Please update to check " - "inSignalContext and/or use LogMessageVerbSigSafe() or " - "ErrorFSigSafe(). The offending log format message is:\n" - "%s\n", format); - return; + if (warned < 3) { + BUG_WARN_MSG(inSignalContext, + "Warning: attempting to log data in a signal unsafe " + "manner while in signal context.\nPlease update to check " + "inSignalContext and/or use LogMessageVerbSigSafe() or " + "ErrorFSigSafe().\nThe offending log format message is:\n" + "%s\n", format); + warned++; + if (warned == 3) + LogMessageVerbSigSafe(X_WARNING, -1, "Warned %u times about sigsafe logging. Will be quiet now.\n", warned); + } } type_str = LogMessageTypeVerbString(type, verb); @@ -556,6 +571,7 @@ void LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format, va_list msg_args, const char *hdr_format, va_list hdr_args) { + static unsigned int warned; const char *type_str; char buf[1024]; const size_t size = sizeof(buf); @@ -563,13 +579,17 @@ LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format, size_t len = 0; if (inSignalContext) { - BUG_WARN_MSG(inSignalContext, - "Warning: attempting to log data in a signal unsafe " - "manner while in signal context. Please update to check " - "inSignalContext and/or use LogMessageVerbSigSafe(). The " - "offending header and log message formats are:\n%s %s\n", - hdr_format, msg_format); - return; + if (warned < 3) { + BUG_WARN_MSG(inSignalContext, + "Warning: attempting to log data in a signal unsafe " + "manner while in signal context.\nPlease update to check " + "inSignalContext and/or use LogMessageVerbSigSafe().\nThe " + "offending header and log message formats are:\n%s %s\n", + hdr_format, msg_format); + warned++; + if (warned == 3) + LogMessageVerbSigSafe(X_WARNING, -1, "Warned %u times about sigsafe logging. Will be quiet now.\n", warned); + } } type_str = LogMessageTypeVerbString(type, verb); diff --git a/os/utils.c b/os/utils.c index 947f8673a..04bcbc61f 100644 --- a/os/utils.c +++ b/os/utils.c @@ -1924,6 +1924,20 @@ xstrtokenize(const char *str, const char *separators) return NULL; } +/* Format a signed number into a string in a signal safe manner. The string + * should be at least 21 characters in order to handle all int64_t values. + */ +void +FormatInt64(int64_t num, char *string) +{ + if (num < 0) { + string[0] = '-'; + num *= -1; + string++; + } + FormatUInt64(num, string); +} + /* Format a number into a string in a signal safe manner. The string should be * at least 21 characters in order to handle all uint64_t values. */ void diff --git a/test/signal-logging.c b/test/signal-logging.c index 8aab0dd58..3206ddefa 100644 --- a/test/signal-logging.c +++ b/test/signal-logging.c @@ -26,6 +26,7 @@ #endif #include <stdint.h> +#include <unistd.h> #include "assert.h" #include "misc.h" @@ -35,6 +36,26 @@ struct number_format_test { char hex_string[17]; }; +struct signed_number_format_test { + int64_t number; + char string[21]; +}; + +static Bool +check_signed_number_format_test(const struct signed_number_format_test *test) +{ + char string[21]; + + FormatInt64(test->number, string); + if(strncmp(string, test->string, 21) != 0) { + fprintf(stderr, "Failed to convert %jd to decimal string (%s vs %s)\n", + test->number, test->string, string); + return FALSE; + } + + return TRUE; +} + static Bool check_number_format_test(const struct number_format_test *test) { @@ -57,11 +78,11 @@ check_number_format_test(const struct number_format_test *test) return TRUE; } -static Bool +static void number_formatting(void) { int i; - struct number_format_test tests[] = { + struct number_format_test unsigned_tests[] = { { /* Zero */ 0, "0", @@ -99,17 +120,190 @@ number_formatting(void) }, }; - for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) - if (!check_number_format_test(tests + i)) - return FALSE; + struct signed_number_format_test signed_tests[] = { + { /* Zero */ + 0, + "0", + }, + { /* Single digit number */ + 5, + "5", + }, + { /* Two digit decimal number */ + 12, + "12", + }, + { /* Two digit hex number */ + 37, + "37", + }, + { /* Large < 32 bit number */ + 0xC90B2, + "823474", + }, + { /* Large > 32 bit number */ + 0x15D027BF211B37A, + "98237498237498234", + }, + { /* Maximum 64-bit signed number */ + 0x7FFFFFFFFFFFFFFF, + "9223372036854775807", + }, + { /* Single digit number */ + -1, + "-1", + }, + { /* Two digit decimal number */ + -12, + "-12", + }, + { /* Large < 32 bit number */ + -0xC90B2, + "-823474", + }, + { /* Large > 32 bit number */ + -0x15D027BF211B37A, + "-98237498237498234", + }, + { /* Maximum 64-bit number */ + -0x7FFFFFFFFFFFFFFF, + "-9223372036854775807", + }, + }; + + for (i = 0; i < sizeof(unsigned_tests) / sizeof(unsigned_tests[0]); i++) + assert(check_number_format_test(unsigned_tests + i)); - return TRUE; + for (i = 0; i < sizeof(unsigned_tests) / sizeof(signed_tests[0]); i++) + assert(check_signed_number_format_test(signed_tests + i)); +} + +static void logging_format(void) +{ + const char *log_file_path = "/tmp/Xorg-logging-test.log"; + const char *str = "%s %d %u %% %p %i"; + char buf[1024]; + int i; + unsigned int ui; + FILE *f; + char read_buf[2048]; + char *logmsg; + uintptr_t ptr; + + /* set up buf to contain ".....end" */ + memset(buf, '.', sizeof(buf)); + strcpy(&buf[sizeof(buf) - 4], "end"); + + LogInit(log_file_path, NULL); + assert(f = fopen(log_file_path, "r")); + +#define read_log_msg(msg) \ + fgets(read_buf, sizeof(read_buf), f); \ + msg = strchr(read_buf, ']') + 2; /* advance past [time.stamp] */ + + /* boring test message */ + 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" */ +#pragma GCC diagnostic ignored "-Wformat-security" + LogMessageVerbSigSafe(X_ERROR, -1, buf); +#pragma GCC diagnostic pop "-Wformat-security" + read_log_msg(logmsg); + assert(strcmp(&logmsg[strlen(logmsg) - 3], "en\n") == 0); + + /* same thing, this time as string substitution */ + 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 */ + LogMessageVerbSigSafe(X_ERROR, -1, "%s\n", str); + read_log_msg(logmsg); + assert(strcmp(logmsg, "(EE) %s %d %u %% %p %i\n") == 0); + + /* string substitution */ + LogMessageVerbSigSafe(X_ERROR, -1, "%s\n", "substituted string"); + read_log_msg(logmsg); + assert(strcmp(logmsg, "(EE) substituted string\n") == 0); + + /* number substitution */ + ui = 0; + do { + char expected[30]; + sprintf(expected, "(EE) %u\n", ui); + LogMessageVerbSigSafe(X_ERROR, -1, "%u\n", ui); + read_log_msg(logmsg); + assert(strcmp(logmsg, expected) == 0); + if (ui == 0) + ui = 1; + else + ui <<= 1; + } while(ui); + + /* signed number substitution */ + i = 0; + do { + char expected[30]; + sprintf(expected, "(EE) %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); + LogMessageVerbSigSafe(X_ERROR, -1, "%d\n", i | INT_MIN); + read_log_msg(logmsg); + assert(strcmp(logmsg, expected) == 0); + + if (i == 0) + i = 1; + else + i <<= 1; + } while(i > INT_MIN); + + /* hex number substitution */ + ui = 0; + do { + char expected[30]; + sprintf(expected, "(EE) %x\n", ui); + LogMessageVerbSigSafe(X_ERROR, -1, "%x\n", ui); + read_log_msg(logmsg); + assert(strcmp(logmsg, expected) == 0); + if (ui == 0) + ui = 1; + else + ui <<= 1; + } while(ui); + + /* pointer substitution */ + /* we print a null-pointer differently to printf */ + LogMessageVerbSigSafe(X_ERROR, -1, "%p\n", NULL); + read_log_msg(logmsg); + assert(strcmp(logmsg, "(EE) 0x0\n") == 0); + + ptr = 1; + do { + char expected[30]; + sprintf(expected, "(EE) %p\n", (void*)ptr); + LogMessageVerbSigSafe(X_ERROR, -1, "%p\n", (void*)ptr); + read_log_msg(logmsg); + assert(strcmp(logmsg, expected) == 0); + ptr <<= 1; + } while(ptr); + + LogClose(EXIT_NO_ERROR); + unlink(log_file_path); + +#undef read_log_msg } int main(int argc, char **argv) { - int ok = number_formatting(); + number_formatting(); + logging_format(); - return ok ? 0 : 1; + return 0; } |