TapTap/FastSync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

fix: #260 accept --opt=value uniformly and report missing arguments

Browse Source 
- Correct misleading doc comments on set_string_option /
  set_positive_int_option / set_nonneg_int_option (they return 0/-1,
  not true/false).
- find_table_option_with_equals() now matches every OPTION_TABLE value
  option (OPT_STRING/OPT_POS_INT/OPT_NONNEG_INT/OPT_ULL), so forms such
  as --max-size=2G, --min-size=1K, --suffix=.bak, --timeout=30,
  --max-depth=5, --backup-dir=X parse instead of dying as 'Unknown option'.
  --max-size/--min-size now accept rsync-style binary suffixes (0 remains a
  valid 'no limit' byte count). Existing special handling for
  --compress-choice, --compress-level, --modify-window=, --chmod=,
  --skip-compress=, --compress-threads= is preserved.
- Options that require a separate value (-p, --exclude, --include,
  --delta-block, --delta-max, --server-port, --bwlimit, --chunk-size,
  --log-file, --exclude-from, --include-from, -T, --skip-compress,
  --compress-threads) now emit an explicit 'missing argument' diagnostic
  instead of falling through to the generic 'Unknown option' branch when
  given as the final argv entry.
- Add unit tests covering the = forms (--max-size=2G, --min-size=1K,
  --suffix=.bak, --timeout=30, --max-depth=5, --backup-dir=X) and a clean
  'missing argument' (not 'Unknown option') diagnostic for trailing
  --exclude/--server-port/--skip-compress/-T.
This commit is contained in:
TapTap
2026-09-05 12:58:30 +02:00
parent beb681c2dc
commit 1d61a1426e
2 changed files with 159 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/client/client_cli.c
+92 -22
View File
@@ -57,7 +57,7 @@ static bool parse_positive_int(const char* s, int* out_val) {
return true;
}
/* Duplicate a string argument into *dest, freeing the old value. Returns true on success, false on
/* Duplicate a string argument into *dest, freeing the old value. Returns 0 on success, -1 on
* failure. */
static int set_string_option(char** dest, const char* value, const char* option_name) {
char* dup = str_dup(value);
@@ -70,7 +70,7 @@ static int set_string_option(char** dest, const char* value, const char* option_
return 0;
}
/* Parse a string as a positive integer into *dest. Returns true on success, false on error. */
/* Parse a string as a positive integer into *dest. Returns 0 on success, -1 on error. */
static int set_positive_int_option(int* dest, const char* value, const char* option_name) {
if (!parse_positive_int(value, dest)) {
log_message(LOG_LEVEL_ERROR, "%s must be a positive integer", option_name);
@@ -102,7 +102,7 @@ static int set_compression_threads_option(int* dest, const char* value) {
return 0;
}
/* Parse a string as a non-negative integer into *dest. Returns true on success, false on error. */
/* Parse a string as a non-negative integer into *dest. Returns 0 on success, -1 on error. */
static int set_nonneg_int_option(int* dest, const char* value, const char* option_name) {
if (!parse_nonneg_int(value, dest)) {
log_message(LOG_LEVEL_ERROR, "%s must be a non-negative integer", option_name);
@@ -237,7 +237,10 @@ static int parse_ull_arg(const char* val, unsigned long long* out, const char* o
return 0;
}
static int parse_size_arg(const char* value, unsigned long long* out) {
/* Parse a byte count with an optional single-letter binary suffix (K/M/G/T/P/E).
* When allow_zero is false, a bare 0 is rejected (size limits use true, since 0
* means "no limit"). Returns 0 on success, -1 on error. */
static int parse_size_arg_allow_zero(const char* value, unsigned long long* out, bool allow_zero) {
if (!value || *value < '0' || *value > '9')
return -1;
char* end;
@@ -281,12 +284,16 @@ static int parse_size_arg(const char* value, unsigned long long* out) {
return -1;
}
}
if (number == 0 || number > ULLONG_MAX / multiplier)
if ((!allow_zero && number == 0) || number > ULLONG_MAX / multiplier)
return -1;
*out = number * multiplier;
return 0;
}
static int parse_size_arg(const char* value, unsigned long long* out) {
return parse_size_arg_allow_zero(value, out, false);
}
/* Append a duplicated pattern to a growable pattern array. Returns 0 on success, -1 on error. */
static int config_add_pattern(char*** patterns, int* count, const char* value,
const char* optname) {
@@ -450,6 +457,8 @@ static const OptionEntry* find_table_option(const char* arg) {
return NULL;
}
/* Match a "--opt=value" argument against table options that take a value. Flags,
* no-ops, and unsupported options do not accept an inline "=" value. */
static const OptionEntry* find_table_option_with_equals(const char* arg, const char** value) {
const char* equals = strchr(arg, '=');
if (!equals || equals == arg)
@@ -460,8 +469,8 @@ static const OptionEntry* find_table_option_with_equals(const char* arg, const c
if ((strlen(entry->name) == name_len && strncmp(arg, entry->name, name_len) == 0) ||
(entry->alias && strlen(entry->alias) == name_len &&
strncmp(arg, entry->alias, name_len) == 0)) {
if (strcmp(entry->name, "--compress-choice") == 0 ||
strcmp(entry->name, "--compress-level") == 0) {
if (entry->kind == OPT_STRING || entry->kind == OPT_POS_INT ||
entry->kind == OPT_NONNEG_INT || entry->kind == OPT_ULL) {
*value = equals + 1;
return entry;
}
@@ -516,8 +525,13 @@ static int apply_table_option(Config* config, const OptionEntry* entry, const ch
return set_nonneg_int_option((int*)field, value, entry->name);
case OPT_ULL: {
unsigned long long v;
if (parse_ull_arg(value, &v, entry->name) != 0)
/* Size-limit options accept rsync-style suffixes (e.g. --max-size=2G); a
* plain byte count, including 0 ("no limit"), stays valid. */
if (parse_size_arg_allow_zero(value, &v, true) != 0) {
log_message(LOG_LEVEL_ERROR, "%s must be a non-negative size (B, K, M, G, T, P, or E)",
entry->name);
return -1;
}
*(unsigned long long*)field = v;
return 0;
}
@@ -667,22 +681,38 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
config->use_multithreading = true;
config->use_metadata = true;
log_info_message(LOG_INFO_MISC, "Enabled archive mode (-c -m -M)");
} else if (opt_is(argv[i], "-p", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "-p", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (set_positive_int_option(&config->ssh_port, argv[++i], "-p") != 0)
return -1;
if (config->ssh_port > 65535) {
log_message(LOG_LEVEL_ERROR, "SSH port must be 1-65535");
return -1;
}
} else if (opt_is(argv[i], "--exclude", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--exclude", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (config_add_pattern(&config->exclude_patterns, &config->exclude_count, argv[++i],
"--exclude") != 0)
return -1;
} else if (opt_is(argv[i], "--include", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--include", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (config_add_pattern(&config->include_patterns, &config->include_count, argv[++i],
"--include") != 0)
return -1;
} else if (opt_is(argv[i], "--delta-block", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--delta-block", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
unsigned long long val;
if (parse_ull_arg(argv[++i], &val, "--delta-block") != 0)
return -1;
@@ -690,7 +720,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
config->delta_block_size = (uint32_t)val;
else
log_message(LOG_LEVEL_WARNING, "--delta-block value %llu out of range, using default", val);
} else if (opt_is(argv[i], "--delta-max", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--delta-max", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
unsigned long long val;
if (parse_ull_arg(argv[++i], &val, "--delta-max") != 0)
return -1;
@@ -731,7 +765,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
} else if (opt_is(argv[i], "-s", NULL)) {
config->use_chunk_serialization = true;
log_info_message(LOG_INFO_MISC, "Enabled Chunk Serialization");
} else if (opt_is(argv[i], "--server-port", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--server-port", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (!parse_positive_int(argv[++i], &config->server_port)) {
char* escaped = output_escape(argv[i], false);
log_message(LOG_LEVEL_ERROR, "invalid --server-port value: %s",
@@ -743,7 +781,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
log_message(LOG_LEVEL_ERROR, "server port must be 1-65535");
return -1;
}
} else if (opt_is(argv[i], "--bwlimit", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--bwlimit", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
unsigned long long kbps;
if (parse_ull_arg(argv[++i], &kbps, "--bwlimit") != 0)
return -1;
@@ -757,7 +799,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
}
io_set_bwlimit(kbps * 1024);
log_info_message(LOG_INFO_MISC, "Set bandwidth limit to %llu KB/s", kbps);
} else if (opt_is(argv[i], "--chunk-size", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--chunk-size", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
unsigned long long val;
if (parse_ull_arg(argv[++i], &val, "--chunk-size") != 0)
return -1;
@@ -766,7 +812,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
return -1;
}
config->chunk_size = val;
} else if (opt_is(argv[i], "--log-file", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--log-file", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (config->log_file) {
fclose(config->log_file);
config->log_file = NULL;
@@ -788,11 +838,19 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
} else if (opt_is(argv[i], "--stderr", NULL)) {
if (i + 1 >= argc || set_stderr_mode(argv[++i]) != 0)
return -1;
} else if (opt_is(argv[i], "--exclude-from", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--exclude-from", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (read_patterns_from_file(argv[++i], &config->exclude_patterns, &config->exclude_count) !=
0)
return -1;
} else if (opt_is(argv[i], "--include-from", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--include-from", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (read_patterns_from_file(argv[++i], &config->include_patterns, &config->include_count) !=
0)
return -1;
@@ -817,16 +875,28 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
} else if (opt_is(argv[i], "--info", NULL)) {
if (i + 1 >= argc || parse_info_flags(argv[++i], config) != 0)
return -1;
} else if (opt_is(argv[i], "-T", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "-T", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (set_positive_int_option(&config->timeout, argv[++i], "-T") != 0)
return -1;
} else if (strncmp(argv[i], "--skip-compress=", 16) == 0) {
if (parse_skip_compress(config, argv[i] + 16) != 0)
return -1;
} else if (opt_is(argv[i], "--skip-compress", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--skip-compress", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (parse_skip_compress(config, argv[++i]) != 0)
return -1;
} else if (opt_is(argv[i], "--compress-threads", NULL) && i + 1 < argc) {
} else if (opt_is(argv[i], "--compress-threads", NULL)) {
if (i + 1 >= argc) {
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
return -1;
}
if (set_compression_threads_option(&config->compression_threads, argv[++i]) != 0)
return -1;
} else if (opt_is(argv[i], "--checksum-choice", "--cc")) {
tests/test_client_cli.c
+67
View File
@@ -1088,6 +1088,70 @@ static void test_parse_args_rejects_invalid_compression_choice() {
config_delete(cfg);
}
/* Every value-taking table option accepts an inline "--opt=value" form. */
static void test_parse_args_table_equals_size_options() {
Config* cfg = config_create();
char* argv[] = {"fastsync", "--max-size=2G", "--min-size=1K", "/src", "/dst"};
int positional_args[2];
int positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0);
EXPECT_TRUE(cfg->max_size == 2ULL * 1024 * 1024 * 1024);
EXPECT_TRUE(cfg->min_size == 1024ULL);
EXPECT_EQ_INT(positional_count, 2);
config_delete(cfg);
}
static void test_parse_args_table_equals_string_and_int_options() {
Config* cfg = config_create();
char* argv[] = {"fastsync", "--suffix=.bak", "--timeout=30", "--max-depth=5", "/src", "/dst"};
int positional_args[2];
int positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), 0);
EXPECT_EQ_STR(cfg->suffix, ".bak");
EXPECT_EQ_INT(cfg->timeout, 30);
EXPECT_EQ_INT(cfg->max_depth, 5);
EXPECT_EQ_INT(positional_count, 2);
config_delete(cfg);
cfg = config_create();
char* backup_argv[] = {"fastsync", "--backup-dir=/tmp/bak", "/src", "/dst"};
positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 4, backup_argv, positional_args, &positional_count), 0);
EXPECT_EQ_STR(cfg->backup_dir, "/tmp/bak");
config_delete(cfg);
}
/* Options that take a separate value must report "missing argument", not the
* generic "Unknown option", when they are the final argv entry. */
static void test_parse_args_missing_argument_diagnostic() {
static const char* const options[] = {"--exclude", "--server-port", "--skip-compress", "-T"};
for (size_t i = 0; i < sizeof(options) / sizeof(options[0]); i++) {
Config* cfg = config_create();
char* argv[] = {"fastsync", (char*)options[i]};
int positional_args[2];
int positional_count = 0;
FILE* log_file = tmpfile();
char log_buffer[512] = {0};
EXPECT_NOT_NULL(log_file);
log_set_file(log_file);
EXPECT_EQ_INT(parse_args(cfg, 2, argv, positional_args, &positional_count), -1);
fflush(log_file);
rewind(log_file);
EXPECT_TRUE(fread(log_buffer, 1, sizeof(log_buffer) - 1, log_file) > 0);
EXPECT_TRUE(strstr(log_buffer, "missing argument") != NULL);
EXPECT_TRUE(strstr(log_buffer, "Unknown option") == NULL);
log_set_file(NULL);
fclose(log_file);
config_delete(cfg);
}
}
void test_client_cli() {
test_validate_config_required_paths();
test_validate_config_incompatible_options();
@@ -1155,6 +1219,9 @@ void test_client_cli() {
test_parse_args_compression_alias_equals();
test_parse_args_rejects_invalid_compression_level_equals();
test_parse_args_rejects_invalid_compression_choice();
test_parse_args_table_equals_size_options();
test_parse_args_table_equals_string_and_int_options();
test_parse_args_missing_argument_diagnostic();
test_parse_args_partial_progress();
test_parse_args_checksum_choice_aliases();
test_parse_args_checksum_choice_requires_value();