Addressed the issues raised by splint and flawfinder.

This commit is contained in:
Andrew Wood
2023-09-10 23:04:43 +01:00
parent b0a67878bb
commit 896727b664
5 changed files with 164 additions and 79 deletions
+7 -7
View File
@@ -88,7 +88,7 @@ struct pvstate_s {
* Input files *
***************/
unsigned int input_file_count; /* number of input files */
const char **input_files; /* input files (0=first) */
/*@only@*/ /*@null@*/ char **input_files; /* input files */
/*******************
* Program control *
@@ -120,14 +120,14 @@ struct pvstate_s {
unsigned int height; /* screen height */
bool width_set_manually; /* width was set manually, not detected */
bool height_set_manually; /* height was set manually, not detected */
/*@null@*/ char *name; /* display name */
/*@only@*/ /*@null@*/ char *name; /* display name */
char default_format[PV_SIZEOF_DEFAULT_FORMAT]; /* default format string */
char *format_string; /* output format string */
/*@only@*/ /*@null@*/ char *format_string; /* output format string */
/******************
* Program status *
******************/
const char *program_name; /* program name for error reporting */
/*@only@*/ char *program_name; /* program name for error reporting */
char cwd[PV_SIZEOF_CWD]; /* current working directory for relative path */
int current_input_file; /* index of current file being read */
int exit_status; /* exit status to give (0=OK) */
@@ -160,15 +160,15 @@ struct pvstate_s {
long double prev_trans;
/* Keep track of progress over last intervals to compute current average rate. */
pvhistory_t *history; /* state at previous intervals (circular buffer) */
int history_len; /* total size */
/*@null@*/ pvhistory_t *history; /* state at previous intervals (circular buffer) */
unsigned int history_len; /* total size */
int history_interval; /* seconds between each history entry */
int history_first;
int history_last;
long double current_avg_rate; /* current average rate over last history intervals */
unsigned long long initial_offset;
char *display_buffer;
/*@only@*/ char *display_buffer;
long display_buffer_size;
int lastoutput_length; /* number of last-output bytes to show */
unsigned char lastoutput_buffer[PV_SIZEOF_LASTOUTPUT_BUFFER];
+6
View File
@@ -78,6 +78,12 @@ extern int pv_snprintf(char *, size_t, const char *, ...);
*/
extern size_t pv_strlcat(char *, const char *, size_t);
/*
* Allocate and return a duplicate of a \0-terminated string, ensuring that
* the duplicate is also \0-terminated. Returns NULL on error.
*/
/*@null@ */ /*@only@ */ extern char *pv_strdup(const char *);
/*
* Functions relating to elapsed time.
*/
+3 -38
View File
@@ -25,41 +25,6 @@ void display_help(void);
void display_version(void);
/*
* Allocate a duplicate of a \0-terminated string.
*/
static /*@null@ */
/*@only@ */
char *xstrdup(const char *original)
{
size_t length;
char *duplicate;
if (NULL == original) {
errno = EINVAL;
return NULL;
}
length = strlen(original); /* flawfinder: ignore */
/*
* flawfinder rationale: the original string is explicitly required
* to be \0 terminated.
*/
duplicate = calloc(1, 1 + length);
if (NULL == duplicate)
return NULL;
memcpy(duplicate, original, length); /* flawfinder: ignore */
/*
* flawfinder rationale: the buffer is explicitly allocated to be
* large enough.
*/
duplicate[length] = '\0';
return duplicate;
}
/*
* Free an opts_t object.
*/
@@ -444,7 +409,7 @@ opts_t opts_parse(unsigned int argc, char **argv)
opts->height_set_manually = opts->height == 0 ? false : true;
break;
case 'N':
opts->name = xstrdup(optarg);
opts->name = pv_strdup(optarg);
if (NULL == opts->name) {
fprintf(stderr, "%s: -N: %s\n", opts->program_name, strerror(errno));
opts_free(opts);
@@ -484,7 +449,7 @@ opts_t opts_parse(unsigned int argc, char **argv)
opts->remote = pv_getnum_ui(optarg);
break;
case 'P':
opts->pidfile = xstrdup(optarg);
opts->pidfile = pv_strdup(optarg);
if (NULL == opts->pidfile) {
fprintf(stderr, "%s: -P: %s\n", opts->program_name, strerror(errno));
opts_free(opts);
@@ -492,7 +457,7 @@ opts_t opts_parse(unsigned int argc, char **argv)
}
break;
case 'F':
opts->format = xstrdup(optarg);
opts->format = pv_strdup(optarg);
if (NULL == opts->format) {
fprintf(stderr, "%s: -F: %s\n", opts->program_name, strerror(errno));
opts_free(opts);
+110 -34
View File
@@ -24,10 +24,17 @@ static void pv_alloc_history(pvstate_t state)
free(state->history);
state->history = NULL;
state->history = calloc(state->history_len, sizeof(state->history[0]));
state->history = calloc((size_t) (state->history_len), sizeof(state->history[0]));
if (NULL == state->history) {
/*@-mustfreefresh@ */
/*
* splint note: the gettext calls made by _() cause memory
* leak warnings, but in this case it's unavoidable, and
* mitigated by the fact we only translate each string once.
*/
fprintf(stderr, "%s: %s: %s\n", state->program_name,
_("history structure allocation failed"), strerror(errno));
/*@+mustfreefresh@ */
return;
}
@@ -45,8 +52,19 @@ pvstate_t pv_state_alloc(const char *program_name)
state = calloc(1, sizeof(*state));
if (NULL == state)
return NULL;
memset(state, 0, sizeof(*state));
/* splint 3.1.2 thinks this is required for some reason. */
if (NULL != state->program_name) {
free(state->program_name);
}
state->program_name = pv_strdup(program_name);
if (NULL == state->program_name) {
free(state);
return NULL;
}
state->program_name = program_name;
state->watch_pid = 0;
state->watch_fd = -1;
#ifdef HAVE_IPC
@@ -88,6 +106,10 @@ void pv_state_free(pvstate_t state)
if (0 == state)
return;
if (NULL != state->program_name)
free(state->program_name);
state->program_name = NULL;
if (NULL != state->display_buffer)
free(state->display_buffer);
state->display_buffer = NULL;
@@ -110,6 +132,18 @@ void pv_state_free(pvstate_t state)
free(state->history);
state->history = NULL;
if (NULL != state->input_files) {
unsigned int file_idx;
for (file_idx = 0; file_idx < state->input_file_count; file_idx++) {
/*@-unqualifiedtrans@ */
free(state->input_files[file_idx]);
/*@+unqualifiedtrans@ */
/* splint: see similar code below. */
}
free(state->input_files);
state->input_files = NULL;
}
free(state);
return;
@@ -124,8 +158,8 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b
{
#define PV_ADDFORMAT(x,y) if (x) { \
if (state->default_format[0] != '\0') \
pv_strlcat(state->default_format, " ", sizeof(state->default_format)); \
pv_strlcat(state->default_format, y, sizeof(state->default_format)); \
(void) pv_strlcat(state->default_format, " ", sizeof(state->default_format)); \
(void) pv_strlcat(state->default_format, y, sizeof(state->default_format)); \
}
state->default_format[0] = '\0';
@@ -139,10 +173,15 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b
PV_ADDFORMAT(eta, "%e");
PV_ADDFORMAT(fineta, "%I");
if (lastwritten > 0) {
char buf[16];
char buf[16]; /* flawfinder: ignore */
memset(buf, 0, sizeof(buf));
(void) pv_snprintf(buf, sizeof(buf), "%%%uA", lastwritten);
PV_ADDFORMAT(lastwritten > 0, buf);
/*
* flawfinder rationale: large enough for string, zeroed
* before use, only written to by pv_snprintf() with the
* right buffer length.
*/
}
if (NULL != state->name) {
@@ -151,7 +190,7 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b
}
if (NULL != name)
state->name = strdup(name);
state->name = pv_strdup(name);
state->reparse_display = 1;
}
@@ -165,47 +204,47 @@ void pv_state_force_set(pvstate_t state, bool val)
void pv_state_cursor_set(pvstate_t state, bool val)
{
state->cursor = val;
};
}
void pv_state_numeric_set(pvstate_t state, bool val)
{
state->numeric = val;
};
}
void pv_state_wait_set(pvstate_t state, bool val)
{
state->wait = val;
};
}
void pv_state_delay_start_set(pvstate_t state, double val)
{
state->delay_start = val;
};
}
void pv_state_linemode_set(pvstate_t state, bool val)
{
state->linemode = val;
};
}
void pv_state_bits_set(pvstate_t state, bool bits)
{
state->bits = bits;
};
}
void pv_state_null_terminated_lines_set(pvstate_t state, bool val)
{
state->null_terminated_lines = val;
};
}
void pv_state_no_display_set(pvstate_t state, bool val)
{
state->no_display = val;
};
}
void pv_state_skip_errors_set(pvstate_t state, unsigned int val)
{
state->skip_errors = val;
};
}
void pv_state_error_skip_block_set(pvstate_t state, unsigned long long val)
{
@@ -215,60 +254,60 @@ void pv_state_error_skip_block_set(pvstate_t state, unsigned long long val)
void pv_state_stop_at_size_set(pvstate_t state, bool val)
{
state->stop_at_size = val;
};
}
void pv_state_sync_after_write_set(pvstate_t state, bool val)
{
state->sync_after_write = val;
};
}
void pv_state_direct_io_set(pvstate_t state, bool val)
{
state->direct_io = val;
state->direct_io_changed = true;
};
}
void pv_state_discard_input_set(pvstate_t state, bool val)
{
state->discard_input = val;
};
}
void pv_state_rate_limit_set(pvstate_t state, unsigned long long val)
{
state->rate_limit = val;
};
}
void pv_state_target_buffer_size_set(pvstate_t state, unsigned long long val)
{
state->target_buffer_size = val;
};
}
void pv_state_no_splice_set(pvstate_t state, bool val)
{
state->no_splice = val;
};
}
void pv_state_size_set(pvstate_t state, unsigned long long val)
{
state->size = val;
};
}
void pv_state_interval_set(pvstate_t state, double val)
{
state->interval = val;
};
}
void pv_state_width_set(pvstate_t state, unsigned int val, bool was_set_manually)
{
state->width = val;
state->width_set_manually = was_set_manually;
};
}
void pv_state_height_set(pvstate_t state, unsigned int val, bool was_set_manually)
{
state->height = val;
state->height_set_manually = was_set_manually;
};
}
void pv_state_name_set(pvstate_t state, /*@null@ */ const char *val)
{
@@ -277,8 +316,8 @@ void pv_state_name_set(pvstate_t state, /*@null@ */ const char *val)
state->name = NULL;
}
if (NULL != val)
state->name = strdup(val);
};
state->name = pv_strdup(val);
}
void pv_state_format_string_set(pvstate_t state, /*@null@ */ const char *val)
{
@@ -287,18 +326,18 @@ void pv_state_format_string_set(pvstate_t state, /*@null@ */ const char *val)
state->format_string = NULL;
}
if (NULL != val)
state->format_string = strdup(val);
};
state->format_string = pv_strdup(val);
}
void pv_state_watch_pid_set(pvstate_t state, unsigned int val)
{
state->watch_pid = val;
};
}
void pv_state_watch_fd_set(pvstate_t state, int val)
{
state->watch_fd = val;
};
}
void pv_state_average_rate_window_set(pvstate_t state, unsigned int val)
{
@@ -312,7 +351,7 @@ void pv_state_average_rate_window_set(pvstate_t state, unsigned int val)
state->history_interval = 1;
}
pv_alloc_history(state);
};
}
/*
@@ -320,8 +359,45 @@ void pv_state_average_rate_window_set(pvstate_t state, unsigned int val)
*/
void pv_state_inputfiles(pvstate_t state, unsigned int input_file_count, const char **input_files)
{
unsigned int file_idx;
if (NULL != state->input_files) {
for (file_idx = 0; file_idx < state->input_file_count; file_idx++) {
/*@-unqualifiedtrans@ */
free(state->input_files[file_idx]);
/*@+unqualifiedtrans@ */
/*
* TODO: find a way to tell splint the array
* contents are "only" and "null" as well as the
* array itself.
*/
}
free(state->input_files);
state->input_files = NULL;
state->input_file_count = 0;
}
state->input_files = calloc((size_t) (input_file_count + 1), sizeof(char *));
if (NULL == state->input_files) {
/*@-mustfreefresh@ *//* see similar _() issue above */
fprintf(stderr, "%s: %s: %s\n", state->program_name, _("file list allocation failed"), strerror(errno));
/*@+mustfreefresh@ */
return;
}
for (file_idx = 0; file_idx < input_file_count; file_idx++) {
/*@-nullstate@ */
state->input_files[file_idx] = pv_strdup(input_files[file_idx]);
if (NULL == state->input_files[file_idx]) {
/*@-mustfreefresh@ *//* see similar _() issue above */
fprintf(stderr, "%s: %s: %s\n", state->program_name,
_("file list allocation failed"), strerror(errno));
/*@+mustfreefresh@ */
return;
}
}
state->input_file_count = input_file_count;
state->input_files = input_files;
}
/*@+nullstate@*/
/* splint: see unqualifiedtrans note by free() above. */
/* EOF */
+38
View File
@@ -12,6 +12,7 @@
#include <stdio.h>
#include <stdarg.h>
#include <string.h>
#include <errno.h>
/*
@@ -113,4 +114,41 @@ size_t pv_strlcat(char *dst, const char *src, size_t dstsize)
#endif
}
/*
* Allocate and return a duplicate of a \0-terminated string, ensuring that
* the duplicate is also \0-terminated. Returns NULL on error.
*/
/*@null@ */
/*@only@ */
char *pv_strdup(const char *original)
{
size_t length;
char *duplicate;
if (NULL == original) {
errno = EINVAL;
return NULL;
}
length = strlen(original); /* flawfinder: ignore */
/*
* flawfinder rationale: the original string is explicitly required
* to be \0 terminated.
*/
duplicate = calloc(1, 1 + length);
if (NULL == duplicate)
return NULL;
memcpy(duplicate, original, length); /* flawfinder: ignore */
/*
* flawfinder rationale: the buffer is explicitly allocated to be
* large enough.
*/
duplicate[length] = '\0';
return duplicate;
}
/* EOF */