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:

49260bb154/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).
This commit is contained in:
Craig Barnes 2024-09-11 20:13:30 +01:00 committed by Daniel Eklöf
parent 31f88e636c
commit d4a1283797
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
8 changed files with 54 additions and 35 deletions

2
csi.c
View file

@ -2127,7 +2127,7 @@ csi_dispatch(struct terminal *term, uint8_t final)
case 'R': { /* XTREPORTCOLORS */ case 'R': { /* XTREPORTCOLORS */
char reply[64]; 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->color_stack.idx, term->color_stack.size);
term_to_slave(term, reply, n); term_to_slave(term, reply, n);
break; break;

22
dcs.c
View file

@ -271,7 +271,7 @@ decrqss_unhook(struct terminal *term)
if (n == 1 && query[0] == 'r') { if (n == 1 && query[0] == 'r') {
/* DECSTBM - Set Top and Bottom Margins */ /* DECSTBM - Set Top and Bottom Margins */
char reply[64]; 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.start + 1,
term->scroll_region.end); term->scroll_region.end);
term_to_slave(term, reply, len); term_to_slave(term, reply, len);
@ -300,7 +300,7 @@ decrqss_unhook(struct terminal *term)
if (a->underline) { if (a->underline) {
if (term->vt.underline.style > UNDERLINE_SINGLE) { if (term->vt.underline.style > UNDERLINE_SINGLE) {
char value[4]; char value[4];
int val_len = size_t val_len =
xsnprintf(value, sizeof(value), "4:%d", term->vt.underline.style); xsnprintf(value, sizeof(value), "4:%d", term->vt.underline.style);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
} else } else
@ -321,7 +321,7 @@ decrqss_unhook(struct terminal *term)
case COLOR_BASE16: { case COLOR_BASE16: {
char value[4]; char value[4];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "%u", value, sizeof(value), "%u",
a->fg >= 8 ? a->fg - 8 + 90 : a->fg + 30); a->fg >= 8 ? a->fg - 8 + 90 : a->fg + 30);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
@ -330,7 +330,7 @@ decrqss_unhook(struct terminal *term)
case COLOR_BASE256: { case COLOR_BASE256: {
char value[16]; 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); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
} }
@ -341,7 +341,7 @@ decrqss_unhook(struct terminal *term)
uint8_t b = a->fg >> 0; uint8_t b = a->fg >> 0;
char value[32]; char value[32];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "38:2::%hhu:%hhu:%hhu", r, g, b); value, sizeof(value), "38:2::%hhu:%hhu:%hhu", r, g, b);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
@ -354,7 +354,7 @@ decrqss_unhook(struct terminal *term)
case COLOR_BASE16: { case COLOR_BASE16: {
char value[4]; char value[4];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "%u", value, sizeof(value), "%u",
a->bg >= 8 ? a->bg - 8 + 100 : a->bg + 40); a->bg >= 8 ? a->bg - 8 + 100 : a->bg + 40);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
@ -363,7 +363,7 @@ decrqss_unhook(struct terminal *term)
case COLOR_BASE256: { case COLOR_BASE256: {
char value[16]; 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); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
} }
@ -374,7 +374,7 @@ decrqss_unhook(struct terminal *term)
uint8_t b = a->bg >> 0; uint8_t b = a->bg >> 0;
char value[32]; char value[32];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "48:2::%hhu:%hhu:%hhu", r, g, b); value, sizeof(value), "48:2::%hhu:%hhu:%hhu", r, g, b);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
@ -388,7 +388,7 @@ decrqss_unhook(struct terminal *term)
case COLOR_BASE256: { case COLOR_BASE256: {
char value[16]; char value[16];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "58:5:%u", term->vt.underline.color); value, sizeof(value), "58:5:%u", term->vt.underline.color);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
@ -400,7 +400,7 @@ decrqss_unhook(struct terminal *term)
uint8_t b = term->vt.underline.color >> 0; uint8_t b = term->vt.underline.color >> 0;
char value[32]; char value[32];
int val_len = xsnprintf( size_t val_len = xsnprintf(
value, sizeof(value), "58:2::%hhu:%hhu:%hhu", r, g, b); value, sizeof(value), "58:2::%hhu:%hhu:%hhu", r, g, b);
append_sgr_attr_n(&reply, &len, value, val_len); append_sgr_attr_n(&reply, &len, value, val_len);
break; break;
@ -432,7 +432,7 @@ decrqss_unhook(struct terminal *term)
mode--; mode--;
char reply[16]; 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); term_to_slave(term, reply, len);
} }

View file

@ -216,6 +216,7 @@ common = static_library(
'log.c', 'log.h', 'log.c', 'log.h',
'char32.c', 'char32.h', 'char32.c', 'char32.h',
'debug.c', 'debug.h', 'debug.c', 'debug.h',
'macros.h',
'xmalloc.c', 'xmalloc.h', 'xmalloc.c', 'xmalloc.h',
'xsnprintf.c', 'xsnprintf.h' 'xsnprintf.c', 'xsnprintf.h'
) )

View file

@ -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]; 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); reply, sizeof(reply), "\033]99;i=%s;%s\033\\", id, button_nr);
term_to_slave(term, reply, n); 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"; const char *id = notif->id != NULL ? notif->id : "0";
char reply[7 + strlen(id) + 1 + 7 + 1 + 2 + 1]; 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); reply, sizeof(reply), "\033]99;i=%s:p=close;\033\\", id);
term_to_slave(term, reply, n); term_to_slave(term, reply, n);
} }

2
osc.c
View file

@ -739,7 +739,7 @@ kitty_notification(struct terminal *term, char *string)
const char *terminator = term->vt.osc.bel ? "\a" : "\033\\"; const char *terminator = term->vt.osc.bel ? "\a" : "\033\\";
char reply[128]; char reply[128];
int n = xsnprintf( size_t n = xsnprintf(
reply, sizeof(reply), 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", "\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); reply_id, p_caps, a_caps, when_caps, u_caps, terminator);

View file

@ -4302,7 +4302,7 @@ term_send_size_notification(struct terminal *term)
const int width = term->width - term->margins.left - term->margins.right; const int width = term->width - term->margins.left - term->margins.right;
char buf[128]; char buf[128];
const int n = xsnprintf( const size_t n = xsnprintf(
buf, sizeof(buf), "\033[48;%d;%d;%d;%dt", buf, sizeof(buf), "\033[48;%d;%d;%d;%dt",
term->rows, term->cols, height, width); term->rows, term->cols, height, width);
term_to_slave(term, buf, n); term_to_slave(term, buf, n);

View file

@ -1,32 +1,51 @@
#include "xsnprintf.h" #include "xsnprintf.h"
#include <errno.h>
#include <limits.h> #include <limits.h>
#include <stdio.h> #include <stdio.h>
#include "debug.h" #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); int len = vsnprintf(buf, n, format, ap);
if (unlikely(len < 0 || len >= (int)n)) {
/* FATAL_ERROR(__func__, (len < 0) ? errno : ENOBUFS);
* 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);
return (size_t)len; return (size_t)len;
} }
size_t 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_list ap;
va_start(ap, format); va_start(ap, format);

View file

@ -4,5 +4,4 @@
#include <stddef.h> #include <stddef.h>
#include "macros.h" #include "macros.h"
size_t xsnprintf(char *buf, size_t len, const char *fmt, ...) PRINTF(3) NONNULL_ARGS; size_t xsnprintf(char *restrict buf, size_t n, const char *restrict format, ...) PRINTF(3) NONNULL_ARGS;
size_t xvsnprintf(char *buf, size_t len, const char *fmt, va_list ap) VPRINTF(3) NONNULL_ARGS;