Refactor display and calculation functions to separate them out more cleanly, move more information into the state structure instead of repeating it as function parameters, and fix a memory leak in the whole-PID mode of --watchfd.

This commit is contained in:
Andrew Wood
2024-10-04 13:27:17 +01:00
parent a118f110fa
commit 65d568c283
4 changed files with 109 additions and 125 deletions
+2
View File
@@ -3,9 +3,11 @@
* i18n: complete set of German translations supplied by Hartmut Goebel ([#98](https://codeberg.org/a-j-wood/pv/pulls/98))
* fix: resume stopped pipelines when running in the background (part of [#56](https://codeberg.org/a-j-wood/pv/issues/56))
* fix: write UTC timestamps in debugging mode to avoid lockups in signal handlers
* fix: correct a memory leak in "`--watchfd PID`"
* cleanup: removed TODO.md, since it's just an outdated copy of the issue tracker
* cleanup: re-ordered structure members to reduce padding
* cleanup: improved readability of SIGTTOU handling code
* cleanup: refactored to separate display, transfer, and calculation more cleanly
* cleanup: instead of moving stderr when backgrounded, set a suspend-output flag
### 1.8.14 - 7 September 2024
+10 -5
View File
@@ -216,18 +216,20 @@ struct pvstate_s {
long double prev_elapsed_sec; /* elapsed sec at which rate last calculated */
long double prev_rate; /* last calculated instantaneous transfer rate */
long double prev_trans; /* bytes transferred since last rate calculation */
long double prev_trans; /* amount transferred since last rate calculation */
long double current_avg_rate; /* current average rate over last history intervals */
/* Keep track of progress over last intervals to compute current average rate. */
/*@null@*/ struct { /* state at previous intervals (circular buffer) */
long double elapsed_sec; /* time since start of transfer */
off_t total_bytes; /* amount transferred by that time */
off_t total_written; /* amount transferred by that time */
} *history;
size_t history_len; /* total size of history array */
size_t history_first; /* index of oldest entry */
size_t history_last; /* index of newest entry */
off_t prev_total_written; /* total amount transferred when called last time */
int percentage; /* transfer percentage completion */
} calc;
@@ -272,6 +274,7 @@ struct pvstate_s {
*/
struct {
/*@only@*/ /*@null@*/ char *transfer_buffer; /* data transfer buffer */
long double elapsed_seconds; /* how long we have been transferring data for */
size_t buffer_size; /* size of buffer */
size_t read_position; /* amount of data in buffer */
size_t write_position; /* buffered data written */
@@ -279,6 +282,8 @@ struct pvstate_s {
ssize_t to_write; /* max to write this time around */
ssize_t written; /* bytes sent to stdout this time */
off_t total_written; /* total bytes or lines transferred */
/*
* While reading from a file descriptor we keep track of how
* many times in a row we've seen errors
@@ -336,9 +341,9 @@ typedef struct pvwatchfd_s *pvwatchfd_t;
void pv_error(pvstate_t, char *, ...);
int pv_main_loop(pvstate_t);
void pv_calculate_transfer_rate(pvstate_t, long double, off_t, off_t);
bool pv_format(pvstate_t, long double, off_t, off_t);
void pv_display(pvstate_t, long double, off_t, off_t);
void pv_calculate_transfer_rate(pvstate_t, bool);
bool pv_format(pvstate_t, bool);
void pv_display(pvstate_t, bool);
ssize_t pv_transfer(pvstate_t, int, bool *, bool *, off_t, long *);
int pv_next_file(pvstate_t, unsigned int, int);
/*@out@*/ const char *pv_current_file_name(pvstate_t);
+56 -76
View File
@@ -607,8 +607,7 @@ static long bound_long(long x, long min, long max)
* rate, otherwise calulate the average rate from the difference between the
* current position + elapsed time pair, and the oldest pair in the buffer.
*/
static void pv__update_average_rate_history(pvstate_t state, off_t total_bytes, long double elapsed_sec,
long double rate)
static void pv__update_average_rate_history(pvstate_t state, long double rate)
{
size_t first = state->calc.history_first;
size_t last = state->calc.history_last;
@@ -624,7 +623,7 @@ static void pv__update_average_rate_history(pvstate_t state, off_t total_bytes,
* elapsed since the previous call yet.
*/
if ((last_elapsed > 0.0)
&& (elapsed_sec < (last_elapsed + state->control.history_interval)))
&& (state->transfer.elapsed_seconds < (last_elapsed + state->control.history_interval)))
return;
/*
@@ -641,13 +640,13 @@ static void pv__update_average_rate_history(pvstate_t state, off_t total_bytes,
}
}
state->calc.history[last].elapsed_sec = elapsed_sec;
state->calc.history[last].total_bytes = total_bytes;
state->calc.history[last].elapsed_sec = state->transfer.elapsed_seconds;
state->calc.history[last].total_written = state->transfer.total_written;
if (first == last) {
state->calc.current_avg_rate = rate;
} else {
off_t bytes = (state->calc.history[last].total_bytes - state->calc.history[first].total_bytes);
off_t bytes = (state->calc.history[last].total_written - state->calc.history[first].total_written);
long double sec = (state->calc.history[last].elapsed_sec - state->calc.history[first].elapsed_sec);
state->calc.current_avg_rate = (long double) bytes / sec;
}
@@ -655,12 +654,9 @@ static void pv__update_average_rate_history(pvstate_t state, off_t total_bytes,
/*
* Update all calculated transfer state (state->calc), where "elapsed_sec"
* is the seconds elapsed since the transfer started, "bytes_since_last" is
* the number of bytes transferred since the last update, and "total_bytes"
* is the total number of bytes transferred so far.
* Update all calculated transfer state (state->calc).
*
* If "bytes_since_last" is negative, this is the final update, so
* If "final" is true, this is the final update, so
* state->calc.transfer_rate and state->calc.average_rate are given as an
* average over the whole transfer; otherwise they are the current transfer
* rate and current average rate.
@@ -669,15 +665,20 @@ static void pv__update_average_rate_history(pvstate_t state, off_t total_bytes,
* completion if state->control.size is greater than zero, otherwise it will
* increase by 2 each call and wrap at 200.
*/
void pv_calculate_transfer_rate(pvstate_t state, long double elapsed_sec, off_t bytes_since_last, off_t total_bytes)
void pv_calculate_transfer_rate(pvstate_t state, bool final)
{
off_t bytes_since_last;
long double time_since_last, transfer_rate, average_rate;
/* Quick sanity check - state must exist, total_bytes must be >= 0. */
/* Quick sanity check - state must exist. */
if (NULL == state)
return;
if (total_bytes < 0)
return;
bytes_since_last = 0;
if (state->transfer.total_written >= 0) {
bytes_since_last = state->transfer.total_written - state->calc.prev_total_written;
state->calc.prev_total_written = state->transfer.total_written;
}
/*
* In case the time since the last update is very small, we keep
@@ -685,19 +686,19 @@ void pv_calculate_transfer_rate(pvstate_t state, long double elapsed_sec, off_t
* adding to that until a reasonable amount of time has passed to
* avoid rate spikes or division by zero.
*/
time_since_last = elapsed_sec - state->calc.prev_elapsed_sec;
time_since_last = state->transfer.elapsed_seconds - state->calc.prev_elapsed_sec;
if (time_since_last <= 0.01) {
transfer_rate = state->calc.prev_rate;
state->calc.prev_trans += bytes_since_last;
} else {
transfer_rate = ((long double) bytes_since_last + state->calc.prev_trans) / time_since_last;
state->calc.prev_elapsed_sec = elapsed_sec;
state->calc.prev_elapsed_sec = state->transfer.elapsed_seconds;
state->calc.prev_trans = 0;
}
state->calc.prev_rate = transfer_rate;
/* Update history and current average rate for ETA. */
pv__update_average_rate_history(state, total_bytes, elapsed_sec, transfer_rate);
pv__update_average_rate_history(state, transfer_rate);
average_rate = state->calc.current_avg_rate;
/*
@@ -705,13 +706,13 @@ void pv_calculate_transfer_rate(pvstate_t state, long double elapsed_sec, off_t
* recalculate the rate - and the average rate - across the whole
* period of the transfer.
*/
if (bytes_since_last < 0) {
if (final) {
/* Sanity check to avoid division by zero */
if (elapsed_sec < 0.000001)
elapsed_sec = 0.000001;
if (state->transfer.elapsed_seconds < 0.000001)
state->transfer.elapsed_seconds = 0.000001;
average_rate =
(((long double) total_bytes) -
((long double) state->display.initial_offset)) / (long double) elapsed_sec;
(((long double) state->transfer.total_written) -
((long double) state->display.initial_offset)) / (long double) (state->transfer.elapsed_seconds);
transfer_rate = average_rate;
}
@@ -732,7 +733,7 @@ void pv_calculate_transfer_rate(pvstate_t state, long double elapsed_sec, off_t
if (state->calc.percentage > 199)
state->calc.percentage = 0;
} else {
state->calc.percentage = pv__calc_percentage(total_bytes, state->control.size);
state->calc.percentage = pv__calc_percentage(state->transfer.total_written, state->control.size);
}
/* Ensure the percentage is never negative or huge. */
@@ -745,28 +746,18 @@ void pv_calculate_transfer_rate(pvstate_t state, long double elapsed_sec, off_t
/*
* Update state->display.display_buffer with status information formatted
* according to the state held within the given structure, where
* "elapsed_sec" is the seconds elapsed since the transfer started,
* "bytes_since_last" is the number of bytes transferred since the last
* update, and "total_bytes" is the total number of bytes transferred so
* far.
* according to the state held within the given structure.
*
* If "bytes_since_last" is negative, this is the final update so the rate
* is given as an an average over the whole transfer; otherwise the current
* rate is shown.
*
* In line mode, "bytes_since_last" and "total_bytes" are in lines, not bytes.
* If "final" is true, this is the final update so the rate is given as an
* an average over the whole transfer; otherwise the current rate is shown.
*
* Returns true if the display buffer can be used, false if not.
*
* When returning true, this function will have also set
* state->display.display_string_len to the length of the string in
* state->display.display_buffer, in bytes.
*
* If "total_bytes" is negative, then free the display buffer and return
* false.
*/
bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last, off_t total_bytes)
bool pv_format(pvstate_t state, bool final)
{
long eta;
int static_portion_size;
@@ -779,15 +770,6 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
if (NULL == state)
return false;
/* Negative total transfer - free memory and return false. */
if (total_bytes < 0) {
if (NULL != state->display.display_buffer)
free(state->display.display_buffer);
state->display.display_buffer = NULL;
state->display.display_buffer_size = 0;
return false;
}
/*
* If the display options need reparsing, do so to generate new
* formatting parameters.
@@ -851,17 +833,20 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
numericprefix[0] = '\0';
if (state->display.component[PV_COMPONENT_TIMER].required)
(void) pv_snprintf(numericprefix, sizeof(numericprefix), "%.4Lf ", elapsed_sec);
(void) pv_snprintf(numericprefix, sizeof(numericprefix), "%.4Lf ",
state->transfer.elapsed_seconds);
if (state->display.component[PV_COMPONENT_BYTES].required) {
if (state->control.bits) {
(void) pv_snprintf(state->display.display_buffer,
state->display.display_buffer_size,
"%.99s%lld\n", numericprefix, (long long) (8 * total_bytes));
"%.99s%lld\n", numericprefix,
(long long) (8 * state->transfer.total_written));
} else {
(void) pv_snprintf(state->display.display_buffer,
state->display.display_buffer_size,
"%.99s%lld\n", numericprefix, (long long) total_bytes);
"%.99s%lld\n", numericprefix,
(long long) (state->transfer.total_written));
}
} else {
(void) pv_snprintf(state->display.display_buffer,
@@ -932,10 +917,10 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
/*@-mustfreefresh @ */
if (state->control.bits && !state->control.linemode) {
pv__sizestr(component_content, component_buf_size, "%s",
(long double) total_bytes * 8, "", _("b"), count_type);
(long double) (state->transfer.total_written * 8), "", _("b"), count_type);
} else {
pv__sizestr(component_content, component_buf_size, "%s",
(long double) total_bytes, "", _("B"), count_type);
(long double) (state->transfer.total_written), "", _("B"), count_type);
}
/*@+mustfreefresh @ */
/* splint: we trust gettext() not to really leak memory. */
@@ -948,26 +933,28 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
* does mean that the timer will stop at a 100,000 hours,
* but since that's 11 years, it shouldn't be a problem.
*/
if (elapsed_sec > (long double) 360000000.0L)
elapsed_sec = (long double) 360000000.0L;
if (state->transfer.elapsed_seconds > (long double) 360000000.0L)
state->transfer.elapsed_seconds = (long double) 360000000.0L;
/*
* If the elapsed time is more than a day, include a day count as
* well as hours, minutes, and seconds.
*/
if (elapsed_sec > (long double) 86400.0L) {
if (state->transfer.elapsed_seconds > (long double) 86400.0L) {
(void) pv_snprintf(component_content,
component_buf_size,
"%ld:%02ld:%02ld:%02ld",
((long) elapsed_sec) / 86400,
(((long) elapsed_sec) / 3600) %
24, (((long) elapsed_sec) / 60) % 60, ((long) elapsed_sec) % 60);
((long) (state->transfer.elapsed_seconds)) / 86400,
(((long) (state->transfer.elapsed_seconds)) / 3600) %
24, (((long) (state->transfer.elapsed_seconds)) / 60) % 60,
((long) (state->transfer.elapsed_seconds)) % 60);
} else {
(void) pv_snprintf(component_content,
component_buf_size,
"%ld:%02ld:%02ld",
((long) elapsed_sec) / 3600,
(((long) elapsed_sec) / 60) % 60, ((long) elapsed_sec) % 60);
((long) (state->transfer.elapsed_seconds)) / 3600,
(((long) (state->transfer.elapsed_seconds)) / 60) % 60,
((long) (state->transfer.elapsed_seconds)) % 60);
}
break;
@@ -1005,7 +992,7 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
case PV_COMPONENT_ETA:
/* Estimated time remaining until completion - if size is known. */
eta =
pv__seconds_remaining(((off_t) total_bytes - state->display.initial_offset),
pv__seconds_remaining((state->transfer.total_written - state->display.initial_offset),
state->control.size - state->display.initial_offset,
state->calc.current_avg_rate);
@@ -1037,7 +1024,7 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
* If this is the final update, show a blank space where the
* ETA used to be.
*/
if (bytes_since_last < 0) {
if (final) {
size_t erase_idx;
for (erase_idx = 0;
erase_idx < component_buf_size && component_content[erase_idx] != '\0';
@@ -1060,7 +1047,7 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
*/
eta =
pv__seconds_remaining((off_t) (total_bytes - state->display.initial_offset),
pv__seconds_remaining(state->transfer.total_written - state->display.initial_offset,
state->control.size - state->display.initial_offset,
state->calc.current_avg_rate);
@@ -1364,28 +1351,21 @@ bool pv_format(pvstate_t state, long double elapsed_sec, off_t bytes_since_last,
/*
* Output status information on standard error, where "elapsed_sec" is the
* seconds elapsed since the transfer started, "bytes_since_last" is the
* number of bytes transferred since the last update, and "total_bytes" is
* the total number of bytes transferred so far.
* Output status information on standard error.
*
* If "bytes_since_last" is negative, this is the final update so the rate
* is given as an an average over the whole transfer; otherwise the current
* rate is shown.
*
* In line mode, "bytes_since_last" and "total_bytes" are in lines, not
* bytes.
* If "final" is true, this is the final update, so the rate is given as an
* an average over the whole transfer; otherwise the current rate is shown.
*/
void pv_display(pvstate_t state, long double elapsed_sec, off_t bytes_since_last, off_t total_bytes)
void pv_display(pvstate_t state, bool final)
{
if (NULL == state)
return;
pv_sig_checkbg();
pv_calculate_transfer_rate(state, elapsed_sec, bytes_since_last, total_bytes);
pv_calculate_transfer_rate(state, final);
if (!pv_format(state, elapsed_sec, bytes_since_last, total_bytes))
if (!pv_format(state, final))
return;
if (NULL == state->display.display_buffer)
+41 -44
View File
@@ -34,13 +34,12 @@
int pv_main_loop(pvstate_t state)
{
long lineswritten;
off_t total_written, transferred_since_last, cansend;
off_t cansend;
ssize_t written;
long double target;
bool eof_in, eof_out, final_update;
struct timespec start_time, next_update, next_ratecheck, cur_time;
struct timespec init_time, next_remotecheck, transfer_elapsed;
long double elapsed_seconds;
int fd;
unsigned int file_idx;
@@ -50,12 +49,9 @@ int pv_main_loop(pvstate_t state)
* "lineswritten" is the lines written by the last transfer,
* but is only updated in line mode.
*
* "total_written" is the total bytes written since the start,
* or in line mode, the total lines written since the start.
*
* "transferred_since_last" is the bytes written since the last
* display, or in line mode, the lines written since the last
* display.
* "state->transfer.total_written" is the total bytes written since
* the start, or in line mode, the total lines written since the
* start.
*
* The remaining variables are all unchanged by linemode.
*/
@@ -66,9 +62,8 @@ int pv_main_loop(pvstate_t state)
eof_in = false;
eof_out = false;
total_written = 0;
state->transfer.total_written = 0;
lineswritten = 0;
transferred_since_last = 0;
state->display.initial_offset = 0;
memset(&cur_time, 0, sizeof(cur_time));
@@ -187,10 +182,10 @@ int pv_main_loop(pvstate_t state)
* try to write more than we're allowed to.
*/
if ((0 < state->control.size) && (state->control.stop_at_size)) {
if ((state->control.size < (total_written + cansend))
if ((state->control.size < (state->transfer.total_written + cansend))
|| ((0 == cansend)
&& (0 == state->control.rate_limit))) {
cansend = state->control.size - total_written;
cansend = state->control.size - state->transfer.total_written;
if (0 >= cansend) {
debug("%s", "write limit reached (size explicitly set) - setting EOF flags");
eof_in = true;
@@ -215,13 +210,11 @@ int pv_main_loop(pvstate_t state)
}
if (state->control.linemode) {
transferred_since_last += lineswritten;
total_written += lineswritten;
state->transfer.total_written += lineswritten;
if (state->control.rate_limit > 0)
target -= lineswritten;
} else {
transferred_since_last += written;
total_written += written;
state->transfer.total_written += written;
if (state->control.rate_limit > 0)
target -= written;
}
@@ -320,10 +313,7 @@ int pv_main_loop(pvstate_t state)
memset(&transfer_elapsed, 0, sizeof(transfer_elapsed));
pv_elapsedtime_subtract(&transfer_elapsed, &cur_time, &init_time);
elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed);
if (final_update)
transferred_since_last = -1;
state->transfer.elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed);
/* Resize the display, if a resize signal was received. */
if (1 == state->flag.terminal_resized) {
@@ -341,9 +331,7 @@ int pv_main_loop(pvstate_t state)
state->control.height = new_height;
}
pv_display(state, elapsed_seconds, transferred_since_last, total_written);
transferred_since_last = 0;
pv_display(state, final_update);
}
debug("%s: %s=%s, %s=%s", "loop ended", "eof_in", eof_in ? "true" : "false", "eof_out",
@@ -377,10 +365,9 @@ int pv_main_loop(pvstate_t state)
int pv_watchfd_loop(pvstate_t state)
{
struct pvwatchfd_s info;
off_t position_now, total_written, transferred_since_last;
off_t position_now;
struct timespec next_update, cur_time;
struct timespec init_time, next_remotecheck, transfer_elapsed;
long double elapsed_seconds;
bool ended, first_check;
int rc;
@@ -427,8 +414,6 @@ int pv_watchfd_loop(pvstate_t state)
pv_elapsedtime_add_nsec(&next_update, (long long) (1000000000.0 * state->control.interval));
ended = false;
total_written = 0;
transferred_since_last = 0;
first_check = true;
while (!ended) {
@@ -448,12 +433,11 @@ int pv_watchfd_loop(pvstate_t state)
if (position_now < 0) {
ended = true;
} else {
transferred_since_last += position_now - total_written;
total_written = position_now;
if (first_check) {
state->display.initial_offset = position_now;
first_check = false;
}
state->transfer.total_written = position_now;
}
pv_elapsedtime_read(&cur_time);
@@ -490,10 +474,7 @@ int pv_watchfd_loop(pvstate_t state)
*/
pv_elapsedtime_subtract(&transfer_elapsed, &cur_time, &init_time);
elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed);
if (ended)
transferred_since_last = -1;
state->transfer.elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed);
/* Resize the display, if a resize signal was received. */
if (1 == state->flag.terminal_resized) {
@@ -511,9 +492,7 @@ int pv_watchfd_loop(pvstate_t state)
state->control.height = new_height;
}
pv_display(state, elapsed_seconds, transferred_since_last, total_written);
transferred_since_last = 0;
pv_display(state, ended);
}
if (!state->control.numeric)
@@ -677,9 +656,8 @@ int pv_watchpid_loop(pvstate_t state)
displayed_lines = 0;
for (fd = 0; fd < FD_SETSIZE && NULL != info_array; fd++) {
off_t position_now, transferred_since_last;
off_t position_now;
struct timespec init_time, transfer_elapsed;
long double elapsed_seconds;
if (displayed_lines >= (int) (state->control.height))
break;
@@ -697,6 +675,11 @@ int pv_watchpid_loop(pvstate_t state)
if (pv_watchfd_changed(&(info_array[idx]))) {
fd_to_idx[fd] = -1;
info_array[idx].watch_pid = 0;
if (NULL != info_array[idx].state)
pv_state_free(info_array[idx].state);
/*@-mustfreeonly@ *//* not a leak - we've just free()d it. */
info_array[idx].state = NULL;
/*@+mustfreeonly@ */
debug("%s %d: %s", "fd", fd, "removing");
}
continue;
@@ -716,11 +699,15 @@ int pv_watchpid_loop(pvstate_t state)
if (position_now < 0) {
fd_to_idx[fd] = -1;
info_array[idx].watch_pid = 0;
if (NULL != info_array[idx].state)
pv_state_free(info_array[idx].state);
/*@-mustfreeonly@ *//* not a leak - we've just free()d it. */
info_array[idx].state = NULL;
/*@+mustfreeonly@ */
debug("%s %d: %s", "fd", fd, "removing");
continue;
}
transferred_since_last = position_now - info_array[idx].position;
info_array[idx].position = position_now;
memset(&init_time, 0, sizeof(init_time));
@@ -738,18 +725,28 @@ int pv_watchpid_loop(pvstate_t state)
*/
pv_elapsedtime_subtract(&transfer_elapsed, &cur_time, &init_time);
elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed);
if (NULL != info_array[idx].state) {
info_array[idx].state->transfer.elapsed_seconds =
pv_elapsedtime_seconds(&transfer_elapsed);
}
if (displayed_lines > 0) {
debug("%s", "adding newline");
pv_tty_write(state, "\n", 1);
}
debug("%s %d [%d]: %Lf / %Ld / %Ld", "fd", fd, idx, elapsed_seconds, transferred_since_last,
position_now);
if (NULL == info_array[idx].state) {
debug("%s %d [%d]: %s / %Ld", "fd", fd, idx, "(null state)", position_now);
} else {
debug("%s %d [%d]: %Lf / %Ld", "fd", fd, idx,
info_array[idx].state->transfer.elapsed_seconds, position_now);
}
pv_display(info_array[idx].state, elapsed_seconds, transferred_since_last, position_now);
displayed_lines++;
if (NULL != info_array[idx].state) {
info_array[idx].state->transfer.total_written = position_now;
pv_display(info_array[idx].state, false);
displayed_lines++;
}
}
/*