diff --git a/autoconf/configure.in b/autoconf/configure.in index 7fc4448..71584cd 100644 --- a/autoconf/configure.in +++ b/autoconf/configure.in @@ -39,7 +39,7 @@ AC_SYS_LARGEFILE AC_HEADER_TIOCGWINSZ AC_CHECK_FUNCS(getopt_long getopt) AC_CHECK_HEADERS(getopt.h) -AC_CHECK_FUNCS(memcpy basename snprintf) +AC_CHECK_FUNCS(memcpy basename vsnprintf) AC_CHECK_HEADERS(limits.h) AC_ARG_ENABLE(debugging, diff --git a/autoconf/header.in b/autoconf/header.in index 355cac4..d137249 100644 --- a/autoconf/header.in +++ b/autoconf/header.in @@ -60,9 +60,6 @@ /* Define to 1 if you have the header file. */ #undef HAVE_MINIX_CONFIG_H -/* Define to 1 if you have the `snprintf' function. */ -#undef HAVE_SNPRINTF - /* Define to 1 if you have the `splice' function. */ #undef HAVE_SPLICE @@ -99,6 +96,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_UNISTD_H +/* Define to 1 if you have the `vsnprintf' function. */ +#undef HAVE_VSNPRINTF + /* Define to 1 if you have the header file. */ #undef HAVE_WCHAR_H diff --git a/doc/NEWS.md b/doc/NEWS.md index d1c1774..064a3d7 100644 --- a/doc/NEWS.md +++ b/doc/NEWS.md @@ -1,12 +1,13 @@ 0.0.20230722-UNRELEASED - * cleanup: Rewrote configure.in as per suggestions in newer autoconf manuals - * cleanup: replaced header.in with one generated by autoheader, moving custom logic to a separate header file "config-aux.h" + * cleanup: Rewrote `configure.in` as per suggestions in newer "`autoconf`" manuals + * cleanup: replaced `header.in` with one generated by "`autoheader`", moving custom logic to a separate header file "`config-aux.h`" * cleanup: added copyright notice to all source files as per GNU standards * cleanup: changed "`--version`" output to conform to GNU standards * cleanup: replaced backticks with `$()` in all shell scripts that did not come from elsewhere * cleanup: extended the "`make test`" mechanism to allow certain tests to be skipped on platforms that cannot support them * cleanup: improved the output formatting of "`make test`" * cleanup: skip test 10 (for *SIGPIPE*) if GNU "`head`" is not available, so "`make test`" on stock OpenBSD 7.3 works + * cleanup: replace all calls to `sprintf()` and `snprintf()` with a new wrapper function `pv_snprintf()` to centralise compatibility changes 1.7.0 - 17 July 2023 * dropped: support for Red Hat Enterprise Linux and its derivatives has been dropped; removed the RPM spec file, and will no longer build binaries diff --git a/src/include/pv.h b/src/include/pv.h index 2d4705b..e354f68 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -9,6 +9,8 @@ #ifndef _PV_H #define _PV_H 1 +#include + #ifdef __cplusplus extern "C" { #endif @@ -53,6 +55,12 @@ extern unsigned long long pv_getnum_ull(const char *); */ extern int pv_getnum_check(const char *, pv_numtype_t); +/* + * Wrapper for sprintf(), with less safe fallbacks for systems without that + * function. + */ +extern int pv_snprintf(char *, size_t, const char *, ...); + /* * Main PV functions. */ diff --git a/src/main/help.c b/src/main/help.c index e53ddb3..4e5884e 100644 --- a/src/main/help.c +++ b/src/main/help.c @@ -7,6 +7,7 @@ */ #include "config.h" +#include "pv.h" #include #include #include @@ -109,6 +110,7 @@ void display_help(void) }; unsigned int i, col1max = 0, tw = 77; char *optbuf; + size_t optbuf_size; printf(_("Usage: %s [OPTION] [FILE]..."), PROGRAM_NAME); printf("\n%s\n\n", @@ -138,7 +140,8 @@ void display_help(void) col1max++; - optbuf = malloc((size_t) (col1max + 16)); + optbuf_size = 16 + col1max; + optbuf = malloc(optbuf_size); if (NULL == optbuf) { fprintf(stderr, "%s: %s\n", PROGRAM_NAME, strerror(errno)); exit(EXIT_FAILURE); @@ -162,19 +165,16 @@ void display_help(void) if (description) description = _(description); -#ifdef HAVE_SNPRINTF - (void) snprintf(optbuf, (size_t) (col1max + 15), - "%s%s%s%s%s", optlist[i].optshort, -#else - sprintf(optbuf, "%s%s%s%s%s", optlist[i].optshort, -#endif + (void) pv_snprintf(optbuf, optbuf_size, "%s%s%s%s%s", + optlist[i].optshort, #ifdef HAVE_GETOPT_LONG - optlist[i].optlong ? ", " : "", - optlist[i].optlong ? optlist[i].optlong : "", + optlist[i].optlong ? ", " : "", + optlist[i]. + optlong ? optlist[i].optlong : "", #else - "", "", + "", "", #endif - param ? " " : "", param ? param : ""); + param ? " " : "", param ? param : ""); printf(" %-*s ", (int) (col1max - 2), optbuf); diff --git a/src/main/version.c b/src/main/version.c index fef2798..2aa3972 100644 --- a/src/main/version.c +++ b/src/main/version.c @@ -14,15 +14,19 @@ */ void display_version(void) { - /* GNU standard first line format: program and version only */ + /* GNU standard first line format: program and version only */ printf("%s %s\n", PROGRAM_NAME, VERSION); /* GNU standard second line format - "Copyright" always in English */ - printf("Copyright %s %s\n", - COPYRIGHT_YEAR, COPYRIGHT_HOLDER); - /* GNU standard license line and free software notice */ - printf("%s\n", _("License: Artistic v2.0 ")); - printf("%s\n", _("This is free software: you are free to change and redistribute it.")); - printf("%s\n", _("There is NO WARRANTY, to the extent permitted by law.")); + printf("Copyright %s %s\n", COPYRIGHT_YEAR, COPYRIGHT_HOLDER); + /* GNU standard license line and free software notice */ + printf("%s\n", + _ + ("License: Artistic v2.0 ")); + printf("%s\n", + _ + ("This is free software: you are free to change and redistribute it.")); + printf("%s\n", + _("There is NO WARRANTY, to the extent permitted by law.")); /* Project web site link */ printf("\n%s: <%s>\n", _("Project web site"), PROJECT_HOMEPAGE); } diff --git a/src/pv/cursor.c b/src/pv/cursor.c index 31daad0..4f99a54 100644 --- a/src/pv/cursor.c +++ b/src/pv/cursor.c @@ -103,17 +103,10 @@ static void pv_crs_open_lockfile(pvstate_t state, int fd) tmpdir = "/tmp"; memset(state->crs_lock_file, 0, sizeof(state->crs_lock_file)); -#ifdef HAVE_SNPRINTF - (void) snprintf(state->crs_lock_file, - sizeof(state->crs_lock_file) - 1, - "%s/pv-%s-%i.lock", tmpdir, basename(ttydev), - (int) geteuid()); -#else - (void) sprintf(state->crs_lock_file, - "%.*s/pv-%8s-%i.lock", - sizeof(state->crs_lock_file) - 64, tmpdir, - basename(ttydev), (int) geteuid()); -#endif + (void) pv_snprintf(state->crs_lock_file, + sizeof(state->crs_lock_file), + "%s/pv-%s-%i.lock", tmpdir, basename(ttydev), + (int) geteuid()); /* * Pawel Piatek - not everyone has O_NOFOLLOW, e.g. AIX doesn't @@ -557,12 +550,8 @@ void pv_crs_update(pvstate_t state, char *str) pv_crs_lock(state, STDERR_FILENO); memset(pos, 0, sizeof(pos)); -# ifdef HAVE_SNPRINTF - (void) snprintf(pos, sizeof(pos) - 1, "\033[%u;1H", - state->height); -# else - (void) sprintf(pos, "\033[%u;1H", state->height); -# endif + (void) pv_snprintf(pos, sizeof(pos), "\033[%u;1H", + state->height); write_retry(STDERR_FILENO, pos, strlen(pos)); for (; offs > 0; offs--) { write_retry(STDERR_FILENO, "\n", 1); @@ -586,11 +575,7 @@ void pv_crs_update(pvstate_t state, char *str) y = 1; memset(pos, 0, sizeof(pos)); -#ifdef HAVE_SNPRINTF - (void) snprintf(pos, sizeof(pos) - 1, "\033[%d;1H", y); -#else - (void) sprintf(pos, "\033[%d;1H", y); -#endif + (void) pv_snprintf(pos, sizeof(pos), "\033[%d;1H", y); pv_crs_lock(state, STDERR_FILENO); @@ -628,11 +613,7 @@ void pv_crs_fini(pvstate_t state) y = 1; memset(pos, 0, sizeof(pos)); -#ifdef HAVE_SNPRINTF - (void) snprintf(pos, sizeof(pos) - 1, "\033[%u;1H\n", y); -#else - (void) sprintf(pos, "\033[%u;1H\n", y); -#endif + (void) pv_snprintf(pos, sizeof(pos), "\033[%u;1H\n", y); pv_crs_lock(state, STDERR_FILENO); diff --git a/src/pv/display.c b/src/pv/display.c index 0a0d91f..d4dbcd9 100644 --- a/src/pv/display.c +++ b/src/pv/display.c @@ -245,8 +245,9 @@ static void pv__sizestr(char *buffer, int bufsize, char *format, /* Fix for display of "1.01e+03" instead of "1010" */ if (display_amount > 99.9) { - sprintf(sizestr_buffer, "%4ld%.2s%.16s", - (long) display_amount, si_prefix, suffix); + (void) pv_snprintf(sizestr_buffer, sizeof(sizestr_buffer), + "%4ld%.2s%.16s", (long) display_amount, + si_prefix, suffix); } else { /* * AIX blows up with %4.3Lg%.2s%.16s for some reason, so we @@ -254,16 +255,14 @@ static void pv__sizestr(char *buffer, int bufsize, char *format, */ char str_disp[64]; /* # to get 13.0GB instead of 13GB (#1477) */ - sprintf(str_disp, "%#4.3Lg", display_amount); - sprintf(sizestr_buffer, "%s%.2s%.16s", - str_disp, si_prefix, suffix); + (void) pv_snprintf(str_disp, sizeof(str_disp), "%#4.3Lg", + display_amount); + (void) pv_snprintf(sizestr_buffer, sizeof(sizestr_buffer), + "%s%.2s%.16s", str_disp, si_prefix, + suffix); } -#ifdef HAVE_SNPRINTF - snprintf(buffer, bufsize, format, sizestr_buffer); -#else - sprintf(buffer, format, sizestr_buffer); -#endif + (void) pv_snprintf(buffer, bufsize, format, sizestr_buffer); } @@ -290,7 +289,9 @@ static void pv__format_init(pvstate_t state) memset(state->format, 0, sizeof(state->format)); if (state->name) { - sprintf(state->str_name, "%9.500s:", state->name); + (void) pv_snprintf(state->str_name, + sizeof(state->str_name), "%9.500s:", + state->name); } formatstr = @@ -649,21 +650,31 @@ static char *pv__format(pvstate_t state, numericprefix[0] = 0; if ((state->components_used & PV_DISPLAY_TIMER) != 0) - sprintf(numericprefix, "%.4Lf ", elapsed_sec); + (void) pv_snprintf(numericprefix, + sizeof(numericprefix), "%.4Lf ", + elapsed_sec); if ((state->components_used & PV_DISPLAY_BYTES) != 0) { if (state->bits) { - sprintf(state->display_buffer, - "%.99s%lld\n", numericprefix, - 8 * total_bytes); + (void) pv_snprintf(state->display_buffer, + sizeof + (state->display_buffer), + "%.99s%lld\n", + numericprefix, + 8 * total_bytes); } else { - sprintf(state->display_buffer, - "%.99s%lld\n", numericprefix, - total_bytes); + (void) pv_snprintf(state->display_buffer, + sizeof + (state->display_buffer), + "%.99s%lld\n", + numericprefix, + total_bytes); } } else { - sprintf(state->display_buffer, "%.99s%ld\n", - numericprefix, state->percentage); + (void) pv_snprintf(state->display_buffer, + sizeof(state->display_buffer), + "%.99s%ld\n", numericprefix, + state->percentage); } return state->display_buffer; @@ -704,13 +715,18 @@ static char *pv__format(pvstate_t state, /* Transfer buffer percentage - set up the display string. */ if ((state->components_used & PV_DISPLAY_BUFPERCENT) != 0) { if (state->buffer_size > 0) - sprintf(state->str_bufpercent, "{%3ld%%}", - pv__calc_percentage(state->read_position - - state->write_position, - state->buffer_size)); + (void) pv_snprintf(state->str_bufpercent, + sizeof(state->str_bufpercent), + "{%3ld%%}", + pv__calc_percentage + (state->read_position - + state->write_position, + state->buffer_size)); #ifdef HAVE_SPLICE if (state->splice_used) - strcpy(state->str_bufpercent, "{----}"); + (void) pv_snprintf(state->str_bufpercent, + sizeof(state->str_bufpercent), + "{%s}", "----"); #endif } @@ -729,16 +745,21 @@ static char *pv__format(pvstate_t state, * well as hours, minutes, and seconds. */ if (elapsed_sec > (long double) 86400.0L) { - sprintf(state->str_timer, "%ld:%02ld:%02ld:%02ld", - ((long) elapsed_sec) / 86400, - (((long) elapsed_sec) / 3600) % 24, - (((long) elapsed_sec) / 60) % 60, - ((long) elapsed_sec) % 60); + (void) pv_snprintf(state->str_timer, + sizeof(state->str_timer), + "%ld:%02ld:%02ld:%02ld", + ((long) elapsed_sec) / 86400, + (((long) elapsed_sec) / 3600) % + 24, + (((long) elapsed_sec) / 60) % + 60, ((long) elapsed_sec) % 60); } else { - sprintf(state->str_timer, "%ld:%02ld:%02ld", - ((long) elapsed_sec) / 3600, - (((long) elapsed_sec) / 60) % 60, - ((long) elapsed_sec) % 60); + (void) pv_snprintf(state->str_timer, + sizeof(state->str_timer), + "%ld:%02ld:%02ld", + ((long) elapsed_sec) / 3600, + (((long) elapsed_sec) / 60) % + 60, ((long) elapsed_sec) % 60); } } @@ -801,14 +822,18 @@ static char *pv__format(pvstate_t state, * well as hours, minutes, and seconds. */ if (eta > 86400L) { - sprintf(state->str_eta, - "%.16s %ld:%02ld:%02ld:%02ld", _("ETA"), - eta / 86400, (eta / 3600) % 24, - (eta / 60) % 60, eta % 60); + (void) pv_snprintf(state->str_eta, + sizeof(state->str_eta), + "%.16s %ld:%02ld:%02ld:%02ld", + _("ETA"), eta / 86400, + (eta / 3600) % 24, + (eta / 60) % 60, eta % 60); } else { - sprintf(state->str_eta, "%.16s %ld:%02ld:%02ld", - _("ETA"), eta / 3600, (eta / 60) % 60, - eta % 60); + (void) pv_snprintf(state->str_eta, + sizeof(state->str_eta), + "%.16s %ld:%02ld:%02ld", + _("ETA"), eta / 3600, + (eta / 60) % 60, eta % 60); } /* @@ -869,12 +894,14 @@ static char *pv__format(pvstate_t state, * by time functions. */ struct tm time = *time_ptr; - sprintf(state->str_fineta, "%.16s ", _("ETA")); + (void) pv_snprintf(state->str_fineta, + sizeof(state->str_fineta), + "%.16s ", _("ETA")); strftime(state->str_fineta + strlen(state->str_fineta), sizeof(state->str_fineta) - 1 - - strlen(state->str_fineta), - time_format, &time); + strlen(state->str_fineta), time_format, + &time); } if (!show_eta) { @@ -920,7 +947,8 @@ static char *pv__format(pvstate_t state, state->percentage = 0; if (state->percentage > 100000) state->percentage = 100000; - sprintf(pct, "%2ld%%", state->percentage); + (void) pv_snprintf(pct, sizeof(pct), "%2ld%%", + state->percentage); available_width = state->width - static_portion_size - diff --git a/src/pv/file.c b/src/pv/file.c index 3abf3e5..6e5f685 100644 --- a/src/pv/file.c +++ b/src/pv/file.c @@ -87,8 +87,7 @@ unsigned long long pv_calc_total_size(pvstate_t state) if (0 == strcmp(state->input_files[i], "-")) { fd = open("/dev/stdin", O_RDONLY); } else { - fd = open(state->input_files[i], - O_RDONLY); + fd = open(state->input_files[i], O_RDONLY); } if (fd >= 0) { total += lseek(fd, 0, SEEK_END); diff --git a/src/pv/loop.c b/src/pv/loop.c index 3255ca4..b99a633 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -549,30 +549,21 @@ int pv_watchpid_loop(pvstate_t state) state->format_string ? state-> format_string : state->default_format; if (NULL == strstr(original_format_string, "%N")) { -#ifdef HAVE_SNPRINTF - snprintf(new_format_string, sizeof(new_format_string), -#else - sprintf(new_format_string, -#endif - "%%N %s", original_format_string); + (void) pv_snprintf(new_format_string, + sizeof(new_format_string), "%%N %s", + original_format_string); } else { -#ifdef HAVE_SNPRINTF - snprintf(new_format_string, sizeof(new_format_string), -#else - sprintf(new_format_string, -#endif - "%s", original_format_string); + (void) pv_snprintf(new_format_string, + sizeof(new_format_string), "%s", + original_format_string); } - new_format_string[sizeof(new_format_string)-1] = '\0'; + new_format_string[sizeof(new_format_string) - 1] = '\0'; state_copy.format_string = NULL; -#ifdef HAVE_SNPRINTF - snprintf(state_copy.default_format, - sizeof(state_copy.default_format), -#else - sprintf(state_copy.default_format, -#endif - "%.510s", new_format_string); - state_copy.default_format[sizeof(state_copy.default_format)-1] = '\0'; + (void) pv_snprintf(state_copy.default_format, + sizeof(state_copy.default_format), + "%.510s", new_format_string); + state_copy.default_format[sizeof(state_copy.default_format) - 1] = + '\0'; /* * Get things ready for the main loop. diff --git a/src/pv/state.c b/src/pv/state.c index 18f09a4..aa0801a 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -136,12 +136,7 @@ void pv_state_set_format(pvstate_t state, bool progress, if (lastwritten > 0) { char buf[16]; memset(buf, 0, sizeof(buf)); -#ifdef HAVE_SNPRINTF - (void) snprintf(buf, sizeof(buf) - 1, "%%%uA", - lastwritten); -#else - (void) sprintf(buf, "%%%uA", lastwritten); -#endif + (void) pv_snprintf(buf, sizeof(buf), "%%%uA", lastwritten); PV_ADDFORMAT(lastwritten > 0, buf); } diff --git a/src/pv/string.c b/src/pv/string.c new file mode 100644 index 0000000..89dd51a --- /dev/null +++ b/src/pv/string.c @@ -0,0 +1,52 @@ +/* + * Functions for converting strings to numbers. + * + * Copyright 2023 Andrew Wood + * + * Distributed under the Artistic License v2.0; see `doc/COPYING'. + */ + +#include "config.h" +#include "pv.h" + +#include +#include + +/* + * Wrapper for sprintf(), with less safe fallbacks for systems without that + * function. + * + * Returns -1 if "str" or "format" are NULL or if "size" is 0. + * + * Otherwise, ensures that the buffer "str" is always terminated with a '\0' + * byte, before returning whatever the system's vsnprintf() or vsprintf() + * returned. + */ +int pv_snprintf(char *str, size_t size, const char *format, ...) +{ + va_list ap; + int ret; + + if (NULL == str) + return -1; + if (0 == size) + return -1; + if (NULL == format) + return -1; + + str[0] = '\0'; + + va_start(ap, format); +#ifdef HAVE_VSNPRINTF + ret = vsnprintf(str, size, format, ap); +#else /* ! HAVE_VSNPRINTF */ + ret = vsprintf(str, format, ap); +#endif /* HAVE_VSNPRINTF */ + va_end(ap); + + str[size - 1] = '\0'; + + return ret; +} + +/* EOF */ diff --git a/src/pv/watchpid.c b/src/pv/watchpid.c index 413687b..6a0c8f3 100644 --- a/src/pv/watchpid.c +++ b/src/pv/watchpid.c @@ -152,18 +152,12 @@ int pv_watchfd_info(pvstate_t state, pvwatchfd_t info, int automatic) info->watch_pid, strerror(errno)); return 1; } -#ifdef HAVE_SNPRINTF - snprintf(info->file_fdinfo, sizeof(info->file_fdinfo) - 1, -#else - sprintf(info->file_fdinfo, -#endif - "/proc/%u/fdinfo/%d", info->watch_pid, info->watch_fd); -#ifdef HAVE_SNPRINTF - snprintf(info->file_fd, sizeof(info->file_fd) - 1, -#else - sprintf(info->file_fd, -#endif - "/proc/%u/fd/%d", info->watch_pid, info->watch_fd); + (void) pv_snprintf(info->file_fdinfo, sizeof(info->file_fdinfo), + "/proc/%u/fdinfo/%d", info->watch_pid, + info->watch_fd); + (void) pv_snprintf(info->file_fd, sizeof(info->file_fd), + "/proc/%u/fd/%d", info->watch_pid, + info->watch_fd); memset(info->file_fdpath, 0, sizeof(info->file_fdpath)); if (readlink @@ -325,11 +319,6 @@ int pv_watchpid_scanfds(pvstate_t state, pvstate_t pristine, struct pvwatchfd_s *info_array = NULL; struct pvstate_s *state_array = NULL; -#ifdef HAVE_SNPRINTF - snprintf(fd_dir, sizeof(fd_dir) - 1, "/proc/%u/fd", watch_pid); -#else - sprintf(fd_dir, "/proc/%u/fd", watch_pid); -#endif #ifdef __APPLE__ struct proc_fdinfo *fd_infos = NULL; int fd_infos_count = 0; @@ -341,6 +330,10 @@ int pv_watchpid_scanfds(pvstate_t state, pvstate_t pristine, #else DIR *dptr; struct dirent *d; + + (void) pv_snprintf(fd_dir, sizeof(fd_dir), "/proc/%u/fd", + watch_pid); + dptr = opendir(fd_dir); if (NULL == dptr) return 1; @@ -553,29 +546,23 @@ void pv_watchpid_setname(pvstate_t state, pvwatchfd_t info) max_display_length = (state->width / 2) - 6; if (max_display_length >= path_length) { -#ifdef HAVE_SNPRINTF - snprintf(info->display_name, - sizeof(info->display_name) - 1, -#else - sprintf(info->display_name, -#endif - "%4d:%.498s", info->watch_fd, file_fdpath); + (void) pv_snprintf(info->display_name, + sizeof(info->display_name), + "%4d:%.498s", info->watch_fd, + file_fdpath); } else { int prefix_length, suffix_length; prefix_length = max_display_length / 4; suffix_length = max_display_length - prefix_length - 3; -#ifdef HAVE_SNPRINTF - snprintf(info->display_name, - sizeof(info->display_name) - 1, -#else - sprintf(info->display_name, -#endif - "%4d:%.*s...%.*s", - info->watch_fd, prefix_length, file_fdpath, - suffix_length, - file_fdpath + path_length - suffix_length); + (void) pv_snprintf(info->display_name, + sizeof(info->display_name), + "%4d:%.*s...%.*s", + info->watch_fd, prefix_length, + file_fdpath, suffix_length, + file_fdpath + path_length - + suffix_length); } debug("%s: %d: [%s]", "set name for fd", info->watch_fd,