refactor(parity): address code-quality review findings
- cli: remove UB in --bwlimit scaling (range-check the double product before casting, drop atoi for the +/-1 form) and add huge/boundary unit tests - test: widen the CI throttle wall-clock band to [1.5, 4.5]s with a 2s cross-tolerance so a loaded runner cannot flake it - log: drop the unused LOG_INFO_BACKUP bit; --info=backup is accepted-but- silent like the other rsync-only categories - client_send: remove the duplicate delete_display_path forward declaration - utils: add non-allocating utils_strip_transfer_root and use it from scanner_note_nonreg and delete_display_path (was duplicated logic) - scanner: lstat() instead of stat() when re-reading an empty dir's metadata - file: drop the no-op else-if and the redundant ELOOP arm in file_ensure_directory_secure (symlinks are refused anyway) - format/stats: document literal_data as whole-file accurate (delta upper bound) instead of claiming literal bytes sent - docs: refresh stale protocol 2.26.0 labels to 2.27.0
This commit is contained in:
+31
-7
@@ -23,6 +23,7 @@
|
||||
#include <langinfo.h>
|
||||
#include <limits.h>
|
||||
#include <locale.h>
|
||||
#include <math.h>
|
||||
#include <time.h>
|
||||
#include <signal.h>
|
||||
#include <stdbool.h>
|
||||
@@ -501,6 +502,7 @@ static bool is_accepted_debug_category(const char* name) {
|
||||
|
||||
static bool is_accepted_info_category(const char* name) {
|
||||
static const char* const categories[] = {
|
||||
"backup",
|
||||
"mount",
|
||||
"syms",
|
||||
"symsafe",
|
||||
@@ -628,8 +630,6 @@ static int parse_info_flags(const char* value, Config* config) {
|
||||
flag = LOG_INFO_FLIST;
|
||||
else if (strcmp(name, "nonreg") == 0)
|
||||
flag = LOG_INFO_NONREG;
|
||||
else if (strcmp(name, "backup") == 0)
|
||||
flag = LOG_INFO_BACKUP;
|
||||
else if (strcmp(name, "progress") == 0)
|
||||
flag = LOG_INFO_PROGRESS;
|
||||
else if (is_accepted_info_category(name))
|
||||
@@ -1897,17 +1897,41 @@ static int parse_bwlimit_value(const char* value, unsigned long long* bytes_per_
|
||||
return -1;
|
||||
}
|
||||
|
||||
long long size = 1;
|
||||
long long base = 1;
|
||||
for (int i = 0; i < reps; i++) {
|
||||
if (size > LLONG_MAX / mult) {
|
||||
if (base > LLONG_MAX / mult) {
|
||||
log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
|
||||
return -1;
|
||||
}
|
||||
size *= mult;
|
||||
base *= mult;
|
||||
}
|
||||
size = (long long)((double)size * atof(value));
|
||||
/* rsync multiplies the numeric prefix (atof) by mult^reps in a signed
|
||||
* ssize_t, which is undefined on overflow. Scale in double and range-check
|
||||
* before converting, so a huge value is rejected as "too large" (where
|
||||
* rsync's overflow happens to land on a negative result) without invoking
|
||||
* signed-overflow UB. */
|
||||
double scaled = (double)base * strtod(value, NULL);
|
||||
/* (double)LLONG_MAX rounds up to 2^63, which is itself out of range for the
|
||||
* cast, so reject at >= that bound; LLONG_MIN == -2^63 is exactly
|
||||
* representable and thus castable, so the lower bound stays strict. */
|
||||
if (!isfinite(scaled) || scaled >= (double)LLONG_MAX || scaled < (double)LLONG_MIN) {
|
||||
log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
|
||||
return -1;
|
||||
}
|
||||
long long size = (long long)scaled;
|
||||
if ((*arg == '+' || *arg == '-') && arg[1] == '1' && arg != value) {
|
||||
size += atoi(arg);
|
||||
/* The only form accepted here is "+1"/"-1" (a longer number leaves a
|
||||
trailing byte and is rejected below), so apply the delta directly and
|
||||
guard the one overflow direction. */
|
||||
if (*arg == '+') {
|
||||
if (size == LLONG_MAX) {
|
||||
log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
|
||||
return -1;
|
||||
}
|
||||
size += 1;
|
||||
} else {
|
||||
size -= 1;
|
||||
}
|
||||
arg += 2;
|
||||
}
|
||||
if (*arg != '\0' || size < 0) {
|
||||
|
||||
@@ -229,7 +229,10 @@ static void transfer_stats_note_entry(TransferStats* stats, const File* file) {
|
||||
}
|
||||
|
||||
/* Account for a regular file (or a whole-file append) the receiver actually
|
||||
stored: rsync's transferred-file count and transferred/literal byte totals. */
|
||||
stored: rsync's transferred-file count and transferred/literal byte totals.
|
||||
`literal_data` counts the whole source size, which is exact for a whole-file
|
||||
send but an upper bound for a delta send (the receiver reuses basis blocks
|
||||
the sender never ships); see TransferStats.literal_data in format.h. */
|
||||
static void transfer_stats_note_transferred(TransferStats* stats, const File* file) {
|
||||
if (stats == NULL || file == NULL)
|
||||
return;
|
||||
@@ -940,8 +943,6 @@ static void source_file_destroy(void* item) {
|
||||
}
|
||||
}
|
||||
|
||||
static const char* delete_display_path(const Config* config, const char* path);
|
||||
|
||||
/* Remove only the same regular source file that was sent. */
|
||||
static void remove_transferred_sources(const Config* config, ArrayList* paths) {
|
||||
if (!config->remove_source_files || !paths)
|
||||
@@ -1075,20 +1076,7 @@ static bool receive_stats_record(int fd, ReceiverStats* stats, ArrayList* would_
|
||||
static const char* delete_display_path(const Config* config, const char* path) {
|
||||
if (!config || !path || !config->send_directory)
|
||||
return path;
|
||||
const char* root = config->send_directory;
|
||||
while (*root == '/')
|
||||
root++;
|
||||
const char* rel = path;
|
||||
while (*rel == '/')
|
||||
rel++;
|
||||
size_t root_len = strlen(root);
|
||||
while (root_len > 0 && root[root_len - 1] == '/')
|
||||
root_len--;
|
||||
if (root_len == 0)
|
||||
return rel;
|
||||
if (strncmp(rel, root, root_len) == 0 && (rel[root_len] == '/' || rel[root_len] == '\0'))
|
||||
return rel + root_len + (rel[root_len] == '/' ? 1 : 0);
|
||||
return rel;
|
||||
return utils_strip_transfer_root(path, config->send_directory);
|
||||
}
|
||||
|
||||
/* Send the final STATUS_FINISHED frame and await the receiver's verdict.
|
||||
|
||||
+2
-13
@@ -438,18 +438,7 @@ static void scanner_record_protected(DirectoryScanner* scanner, const char* fs_p
|
||||
static void scanner_note_nonreg(const ScannerOptions* options, const char* fs_path) {
|
||||
if (!options || !options->note_nonreg || !fs_path)
|
||||
return;
|
||||
const char* rel = *fs_path == '/' ? fs_path + 1 : fs_path;
|
||||
const char* root = options->send_directory;
|
||||
if (root != NULL) {
|
||||
while (*root == '/')
|
||||
root++;
|
||||
size_t root_len = strlen(root);
|
||||
while (root_len > 0 && root[root_len - 1] == '/')
|
||||
root_len--;
|
||||
if (root_len > 0 && strncmp(root, rel, root_len) == 0 &&
|
||||
(rel[root_len] == '/' || rel[root_len] == '\0'))
|
||||
rel += root_len + (rel[root_len] == '/' ? 1 : 0);
|
||||
}
|
||||
const char* rel = utils_strip_transfer_root(fs_path, options->send_directory);
|
||||
char* escaped = output_escape(rel, options->eight_bit_output);
|
||||
printf("skipping non-regular file \"%s\"\n", escaped ? escaped : rel);
|
||||
free(escaped);
|
||||
@@ -947,7 +936,7 @@ static bool scanner_emit_empty_dir(DirectoryScanner* scanner, ArrayList* chunk_d
|
||||
if (!scanner->current_path || !scanner->current_rel || scanner->current_rel[0] == '\0')
|
||||
return true;
|
||||
struct stat st;
|
||||
if (stat(scanner->current_path, &st) != 0 || !S_ISDIR(st.st_mode))
|
||||
if (lstat(scanner->current_path, &st) != 0 || !S_ISDIR(st.st_mode))
|
||||
return true;
|
||||
File* dir = scanner_build_dir_file(scanner->current_path, &st, &scanner->options);
|
||||
if (!dir)
|
||||
|
||||
Reference in New Issue
Block a user