From d4a1283797b5a964d92e64c698d65f701a90e1cc Mon Sep 17 00:00:00 2001 From: Craig Barnes Date: Wed, 11 Sep 2024 20:13:30 +0100 Subject: [PATCH] xsnprintf: various improvements related to xvsnprintf() and xsnprintf() Summary of changes: * Make xvsnprintf() static * restrict-qualify pointer arguments (as done by the libc equivalents) * Make comments and spec references more thorough * Remove pointless `n <= INT_MAX` assertion (see comment) * Use FATAL_ERROR() instead of xassert() (since the assertion is inside a shared util function but the caller is responsible for ensuring the condition holds true) * Change some callers to use size_t instead of int for the return value (negative returns are impossible and all subsequent uses are size_t) The updated comments and code were taken (and adapted) from: https://gitlab.com/craigbarnes/dte/-/blob/49260bb154bca0434462a41c061e512540ec2e49/src/util/xsnprintf.c#L6-50 This work was entirely authored by me and I hereby license this contribution under the MIT license (stated explicitly, so that there's no ambiguity w.r.t. the original license). --- csi.c | 2 +- dcs.c | 22 +++++++++++----------- meson.build | 1 + notify.c | 4 ++-- osc.c | 2 +- terminal.c | 2 +- xsnprintf.c | 53 ++++++++++++++++++++++++++++++++++++----------------- xsnprintf.h | 3 +-- 8 files changed, 54 insertions(+), 35 deletions(-) diff --git a/csi.c b/csi.c index 91d42e2a..7c6ea7ca 100644 --- a/csi.c +++ b/csi.c @@ -2127,7 +2127,7 @@ csi_dispatch(struct terminal *term, uint8_t final) case 'R': { /* XTREPORTCOLORS */ char reply[64]; - int n = xsnprintf(reply, sizeof(reply), "\033[?%zu;%zu#Q", + size_t n = xsnprintf(reply, sizeof(reply), "\033[?%zu;%zu#Q", term->color_stack.idx, term->color_stack.size); term_to_slave(term, reply, n); break; diff --git a/dcs.c b/dcs.c index 6a832aed..7518c07c 100644 --- a/dcs.c +++ b/dcs.c @@ -271,7 +271,7 @@ decrqss_unhook(struct terminal *term) if (n == 1 && query[0] == 'r') { /* DECSTBM - Set Top and Bottom Margins */ char reply[64]; - int len = xsnprintf(reply, sizeof(reply), "\033P1$r%d;%dr\033\\", + size_t len = xsnprintf(reply, sizeof(reply), "\033P1$r%d;%dr\033\\", term->scroll_region.start + 1, term->scroll_region.end); term_to_slave(term, reply, len); @@ -300,7 +300,7 @@ decrqss_unhook(struct terminal *term) if (a->underline) { if (term->vt.underline.style > UNDERLINE_SINGLE) { char value[4]; - int val_len = + size_t val_len = xsnprintf(value, sizeof(value), "4:%d", term->vt.underline.style); append_sgr_attr_n(&reply, &len, value, val_len); } else @@ -321,7 +321,7 @@ decrqss_unhook(struct terminal *term) case COLOR_BASE16: { char value[4]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "%u", a->fg >= 8 ? a->fg - 8 + 90 : a->fg + 30); append_sgr_attr_n(&reply, &len, value, val_len); @@ -330,7 +330,7 @@ decrqss_unhook(struct terminal *term) case COLOR_BASE256: { char value[16]; - int val_len = xsnprintf(value, sizeof(value), "38:5:%u", a->fg); + size_t val_len = xsnprintf(value, sizeof(value), "38:5:%u", a->fg); append_sgr_attr_n(&reply, &len, value, val_len); break; } @@ -341,7 +341,7 @@ decrqss_unhook(struct terminal *term) uint8_t b = a->fg >> 0; char value[32]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "38:2::%hhu:%hhu:%hhu", r, g, b); append_sgr_attr_n(&reply, &len, value, val_len); break; @@ -354,7 +354,7 @@ decrqss_unhook(struct terminal *term) case COLOR_BASE16: { char value[4]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "%u", a->bg >= 8 ? a->bg - 8 + 100 : a->bg + 40); append_sgr_attr_n(&reply, &len, value, val_len); @@ -363,7 +363,7 @@ decrqss_unhook(struct terminal *term) case COLOR_BASE256: { char value[16]; - int val_len = xsnprintf(value, sizeof(value), "48:5:%u", a->bg); + size_t val_len = xsnprintf(value, sizeof(value), "48:5:%u", a->bg); append_sgr_attr_n(&reply, &len, value, val_len); break; } @@ -374,7 +374,7 @@ decrqss_unhook(struct terminal *term) uint8_t b = a->bg >> 0; char value[32]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "48:2::%hhu:%hhu:%hhu", r, g, b); append_sgr_attr_n(&reply, &len, value, val_len); break; @@ -388,7 +388,7 @@ decrqss_unhook(struct terminal *term) case COLOR_BASE256: { char value[16]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "58:5:%u", term->vt.underline.color); append_sgr_attr_n(&reply, &len, value, val_len); break; @@ -400,7 +400,7 @@ decrqss_unhook(struct terminal *term) uint8_t b = term->vt.underline.color >> 0; char value[32]; - int val_len = xsnprintf( + size_t val_len = xsnprintf( value, sizeof(value), "58:2::%hhu:%hhu:%hhu", r, g, b); append_sgr_attr_n(&reply, &len, value, val_len); break; @@ -432,7 +432,7 @@ decrqss_unhook(struct terminal *term) mode--; char reply[16]; - int len = xsnprintf(reply, sizeof(reply), "\033P1$r%d q\033\\", mode); + size_t len = xsnprintf(reply, sizeof(reply), "\033P1$r%d q\033\\", mode); term_to_slave(term, reply, len); } diff --git a/meson.build b/meson.build index c6e0e2ef..1b3b01aa 100644 --- a/meson.build +++ b/meson.build @@ -216,6 +216,7 @@ common = static_library( 'log.c', 'log.h', 'char32.c', 'char32.h', 'debug.c', 'debug.h', + 'macros.h', 'xmalloc.c', 'xmalloc.h', 'xsnprintf.c', 'xsnprintf.h' ) diff --git a/notify.c b/notify.c index d1c06fb1..58388b1c 100644 --- a/notify.c +++ b/notify.c @@ -268,7 +268,7 @@ notif_done(struct reaper *reaper, pid_t pid, int status, void *data) } char reply[7 + strlen(id) + 1 + strlen(button_nr) + 2 + 1]; - int n = xsnprintf( + size_t n = xsnprintf( reply, sizeof(reply), "\033]99;i=%s;%s\033\\", id, button_nr); term_to_slave(term, reply, n); } @@ -278,7 +278,7 @@ notif_done(struct reaper *reaper, pid_t pid, int status, void *data) const char *id = notif->id != NULL ? notif->id : "0"; char reply[7 + strlen(id) + 1 + 7 + 1 + 2 + 1]; - int n = xsnprintf( + size_t n = xsnprintf( reply, sizeof(reply), "\033]99;i=%s:p=close;\033\\", id); term_to_slave(term, reply, n); } diff --git a/osc.c b/osc.c index 63b66dd7..0db11811 100644 --- a/osc.c +++ b/osc.c @@ -739,7 +739,7 @@ kitty_notification(struct terminal *term, char *string) const char *terminator = term->vt.osc.bel ? "\a" : "\033\\"; char reply[128]; - int n = xsnprintf( + size_t n = xsnprintf( reply, sizeof(reply), "\033]99;i=%s:p=?;p=%s:a=%s:o=%s:u=%s:c=1:w=1:s=system,silent,error,warn,warning,info,question%s", reply_id, p_caps, a_caps, when_caps, u_caps, terminator); diff --git a/terminal.c b/terminal.c index 7e490418..453798e8 100644 --- a/terminal.c +++ b/terminal.c @@ -4302,7 +4302,7 @@ term_send_size_notification(struct terminal *term) const int width = term->width - term->margins.left - term->margins.right; char buf[128]; - const int n = xsnprintf( + const size_t n = xsnprintf( buf, sizeof(buf), "\033[48;%d;%d;%d;%dt", term->rows, term->cols, height, width); term_to_slave(term, buf, n); diff --git a/xsnprintf.c b/xsnprintf.c index b0e17741..2f6f8493 100644 --- a/xsnprintf.c +++ b/xsnprintf.c @@ -1,32 +1,51 @@ #include "xsnprintf.h" +#include #include #include #include "debug.h" +#include "macros.h" -size_t -xvsnprintf(char *buf, size_t n, const char *format, va_list ap) +/* + * ISO C doesn't require vsnprintf(3) to set errno on failure, but + * POSIX does: + * + * "If an output error was encountered, these functions shall return + * a negative value and set errno to indicate the error." + * + * The mandated errors of interest are: + * + * - EILSEQ: A wide-character code does not correspond to a valid character + * - EOVERFLOW: The value of n is greater than INT_MAX + * - EOVERFLOW: The value to be returned is greater than INT_MAX + * + * ISO C11 states: + * + * "The vsnprintf function returns the number of characters that would + * have been written had n been sufficiently large, not counting the + * terminating null character, or a negative value if an encoding error + * occurred. Thus, the null-terminated output has been completely + * written if and only if the returned value is nonnegative and less + * than n." + * + * See also: + * + * - ISO C11 §7.21.6.12p3 + * - https://pubs.opengroup.org/onlinepubs/9699919799/functions/vsnprintf.html + * - https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html + */ +static size_t +xvsnprintf(char *restrict buf, size_t n, const char *restrict format, va_list ap) { - xassert(n <= INT_MAX); int len = vsnprintf(buf, n, format, ap); - - /* - * ISO C11 §7.21.6.5 states: - * "The snprintf function returns the number of characters that - * would have been written had n been sufficiently large, not - * counting the terminating null character, or a negative value - * if an encoding error occurred. Thus, the null-terminated output - * has been completely written if and only if the returned value - * is nonnegative and less than n." - */ - xassert(len >= 0); - xassert(len < (int)n); - + if (unlikely(len < 0 || len >= (int)n)) { + FATAL_ERROR(__func__, (len < 0) ? errno : ENOBUFS); + } return (size_t)len; } size_t -xsnprintf(char *buf, size_t n, const char *format, ...) +xsnprintf(char *restrict buf, size_t n, const char *restrict format, ...) { va_list ap; va_start(ap, format); diff --git a/xsnprintf.h b/xsnprintf.h index c745ef3e..9463a34a 100644 --- a/xsnprintf.h +++ b/xsnprintf.h @@ -4,5 +4,4 @@ #include #include "macros.h" -size_t xsnprintf(char *buf, size_t len, const char *fmt, ...) PRINTF(3) NONNULL_ARGS; -size_t xvsnprintf(char *buf, size_t len, const char *fmt, va_list ap) VPRINTF(3) NONNULL_ARGS; +size_t xsnprintf(char *restrict buf, size_t n, const char *restrict format, ...) PRINTF(3) NONNULL_ARGS;