diff --git a/doc/NEWS.md b/doc/NEWS.md index 4be2617..580a5de 100644 --- a/doc/NEWS.md +++ b/doc/NEWS.md @@ -5,6 +5,7 @@ * cleanup: added a test to "`make test`" to ensure that "`make install`" installs everything expected * cleanup: replaced *AC_HEADER_TIOCGWINSZ* with *AC_CHECK_HEADERS(sys/ioctl.h)* for better MacOS compatibility ([#74](https://codeberg.org/a-j-wood/pv/issues/74)) * cleanup: with "`--sync`", call `fsync()` instead of `fdatasync()` on incapable systems ([#73](https://codeberg.org/a-j-wood/pv/issues/73)) + * cleanup: keep self-contained copies of name and format string in PV internal state for memory safety * misc: moved from GitHub to Codeberg; updated issue tracking links in documentation 1.7.24 - 30 July 2023 diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index fb31cc1..3fbe010 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -106,9 +106,9 @@ struct pvstate_s { int watch_fd; /* fd to watch */ unsigned int width; /* screen width */ unsigned int height; /* screen height */ - /*@null@*/ const char *name; /* display name */ + /*@null@*/ char *name; /* display name */ char default_format[PV_SIZEOF_DEFAULT_FORMAT]; /* default format string */ - const char *format_string; /* output format string */ + char *format_string; /* output format string */ /****************** * Program status * diff --git a/src/main/options.c b/src/main/options.c index 2b75593..1665865 100644 --- a/src/main/options.c +++ b/src/main/options.c @@ -130,7 +130,7 @@ bool opts_add_file(opts_t opts, const char *filename) * aren't copied anywhere, just the pointers are copied, so make sure the * command line data isn't overwritten or argv[1] free()d or whatever. */ - /*@null@ *//*@only@ */ +/*@null@ */ /*@only@ */ opts_t opts_parse(unsigned int argc, char **argv) { #ifdef HAVE_GETOPT_LONG diff --git a/src/main/remote.c b/src/main/remote.c index b172d51..a669812 100644 --- a/src/main/remote.c +++ b/src/main/remote.c @@ -280,13 +280,11 @@ void pv_remote_check(pvstate_t state) msgbuf.name[sizeof(msgbuf.name) - 1] = '\0'; msgbuf.format[sizeof(msgbuf.format) - 1] = '\0'; - /* TODO: replace strdup(msgbuf.name) with just msgbuf.name once pv_state_* do their own strdup of name */ - pv_state_set_format(state, msgbuf.progress, msgbuf.timer, msgbuf.eta, msgbuf.fineta, msgbuf.rate, msgbuf.average_rate, msgbuf.bytes, msgbuf.bufpercent, - msgbuf.lastwritten, '\0' == msgbuf.name[0] ? NULL : strdup(msgbuf.name)); + msgbuf.lastwritten, '\0' == msgbuf.name[0] ? NULL : msgbuf.name); if (msgbuf.rate_limit > 0) pv_state_rate_limit_set(state, msgbuf.rate_limit); @@ -302,7 +300,7 @@ void pv_remote_check(pvstate_t state) if (msgbuf.height > 0) pv_state_height_set(state, msgbuf.height); if (msgbuf.format[0] != '\0') - pv_state_format_string_set(state, strdup(msgbuf.format)); + pv_state_format_string_set(state, msgbuf.format); } diff --git a/src/pv/state.c b/src/pv/state.c index 6780a2e..ca61bdf 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -92,8 +92,15 @@ void pv_state_free(pvstate_t state) free(state->display_buffer); state->display_buffer = NULL; - /* TODO: after changing ->name to a strdup(), free() it here */ - /* TODO: after changing ->format_string to a strdup(), free() it here */ + if (NULL != state->name) { + free(state->name); + state->name = NULL; + } + + if (NULL != state->format_string) { + free(state->format_string); + state->format_string = NULL; + } if (NULL != state->transfer_buffer) free(state->transfer_buffer); @@ -138,8 +145,14 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b PV_ADDFORMAT(lastwritten > 0, buf); } - /* TODO: free state->name, store strdup(name) */ - state->name = name; + if (NULL != state->name) { + free(state->name); + state->name = NULL; + } + + if (NULL != name) + state->name = strdup(name); + state->reparse_display = 1; } @@ -247,14 +260,22 @@ void pv_state_height_set(pvstate_t state, unsigned int val) void pv_state_name_set(pvstate_t state, /*@null@ */ const char *val) { - /* TODO: free state->name, store strdup(val) */ - state->name = val; + if (NULL != state->name) { + free(state->name); + state->name = NULL; + } + if (NULL != val) + state->name = strdup(val); }; void pv_state_format_string_set(pvstate_t state, /*@null@ */ const char *val) { - /* TODO: free state->format_string, store strdup(val) */ - state->format_string = val; + if (NULL != state->format_string) { + free(state->format_string); + state->format_string = NULL; + } + if (NULL != val) + state->format_string = strdup(val); }; void pv_state_watch_pid_set(pvstate_t state, unsigned int val)