refactor(cli): dedupe server/client option parsing and tighten CLI tests
- server_cli: handle --password-file/--early-input/--iconv via arg_has_value in one place, removing the unreachable duplicate separate-form arms while keeping both --opt VALUE and --opt=VALUE working - client_cli: factor the triplicated --delta-block/--block-size range check into set_delta_block_size(); drop the redundant use_metadata assignment after identity_parse_copy_as (the parser already forces it) - tests: cover both spellings of --iconv/--delta-block, make the archive short-form test actually call parse_args, add delta-block invalid cases
This commit is contained in:
+18
-20
@@ -389,6 +389,21 @@ static int parse_ull_arg(const char* val, unsigned long long* out, const char* o
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Apply a --delta-block/--block-size value (both spellings and both the inline
|
||||
* and separate argument forms share this one range check). An out-of-range
|
||||
* value warns once and leaves the configured default untouched. Returns 0 on
|
||||
* success, -1 on a non-numeric value. */
|
||||
static int set_delta_block_size(Config* config, const char* value) {
|
||||
unsigned long long val;
|
||||
if (parse_ull_arg(value, &val, "--block-size/--delta-block") != 0)
|
||||
return -1;
|
||||
if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX)
|
||||
config->delta_block_size = (uint32_t)val;
|
||||
else
|
||||
log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* 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. */
|
||||
@@ -1094,33 +1109,18 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
|
||||
"--include") != 0)
|
||||
return -1;
|
||||
} else if (strncmp(argv[i], "--delta-block=", 14) == 0) {
|
||||
unsigned long long val;
|
||||
if (parse_ull_arg(argv[i] + 14, &val, "--block-size/--delta-block") != 0)
|
||||
if (set_delta_block_size(config, argv[i] + 14) != 0)
|
||||
return -1;
|
||||
if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX)
|
||||
config->delta_block_size = (uint32_t)val;
|
||||
else
|
||||
log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val);
|
||||
} else if (strncmp(argv[i], "--block-size=", 13) == 0) {
|
||||
unsigned long long val;
|
||||
if (parse_ull_arg(argv[i] + 13, &val, "--block-size/--delta-block") != 0)
|
||||
if (set_delta_block_size(config, argv[i] + 13) != 0)
|
||||
return -1;
|
||||
if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX)
|
||||
config->delta_block_size = (uint32_t)val;
|
||||
else
|
||||
log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val);
|
||||
} else if (opt_is(argv[i], "--delta-block", "--block-size")) {
|
||||
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, "--block-size/--delta-block") != 0)
|
||||
if (set_delta_block_size(config, argv[++i]) != 0)
|
||||
return -1;
|
||||
if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX)
|
||||
config->delta_block_size = (uint32_t)val;
|
||||
else
|
||||
log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val);
|
||||
} else if (opt_is(argv[i], "--delta-max", NULL)) {
|
||||
if (i + 1 >= argc) {
|
||||
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
|
||||
@@ -1431,7 +1431,6 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
|
||||
} else if (strncmp(argv[i], "--copy-as=", 10) == 0) {
|
||||
if (identity_parse_copy_as(config, argv[i] + 10) != 0)
|
||||
return -1;
|
||||
config->use_metadata = true;
|
||||
} else if (opt_is(argv[i], "--copy-as", NULL)) {
|
||||
if (i + 1 >= argc) {
|
||||
log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]);
|
||||
@@ -1439,7 +1438,6 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
|
||||
}
|
||||
if (identity_parse_copy_as(config, argv[++i]) != 0)
|
||||
return -1;
|
||||
config->use_metadata = true;
|
||||
} else if (strncmp(argv[i], "--outbuf=", 9) == 0) {
|
||||
if (set_outbuf_option(config, argv[i] + 9) != 0)
|
||||
return -1;
|
||||
|
||||
+26
-44
@@ -64,6 +64,7 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err,
|
||||
server_cli_options_default(opts);
|
||||
|
||||
for (int i = 1; i < argc; i++) {
|
||||
const char* inline_value = NULL;
|
||||
if (arg_is(argv[i], "--help")) {
|
||||
opts->show_help = true;
|
||||
return 1;
|
||||
@@ -108,18 +109,24 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err,
|
||||
}
|
||||
opts->destination_root = argv[++i];
|
||||
opts->destination_root_set = true;
|
||||
} else if (arg_is(argv[i], "--password-file")) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --password-file");
|
||||
return -1;
|
||||
} else if (arg_has_value(argv[i], "--password-file", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --password-file");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->password_file = argv[++i];
|
||||
} else if (arg_is(argv[i], "--early-input")) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --early-input");
|
||||
return -1;
|
||||
opts->password_file = inline_value;
|
||||
} else if (arg_has_value(argv[i], "--early-input", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --early-input");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->early_input_file = argv[++i];
|
||||
opts->early_input_file = inline_value;
|
||||
} else if (arg_is(argv[i], "--address")) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --address");
|
||||
@@ -146,12 +153,15 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err,
|
||||
opts->no_super = true;
|
||||
} else if (arg_is(argv[i], "--allow-unauthenticated")) {
|
||||
opts->allow_unauthenticated = true;
|
||||
} else if (arg_is(argv[i], "--iconv")) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --iconv");
|
||||
return -1;
|
||||
} else if (arg_has_value(argv[i], "--iconv", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --iconv");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->iconv_spec = argv[++i];
|
||||
opts->iconv_spec = inline_value;
|
||||
} else if (arg_is(argv[i], "-p")) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for -p");
|
||||
@@ -161,7 +171,6 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err,
|
||||
if (parse_port_arg(argv[++i], &opts->port, err, err_size) != 0)
|
||||
return -1;
|
||||
} else {
|
||||
const char* inline_value = NULL;
|
||||
if (arg_has_value(argv[i], "--config", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
@@ -171,33 +180,6 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err,
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->config_path = inline_value;
|
||||
} else if (arg_has_value(argv[i], "--password-file", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --password-file");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->password_file = inline_value;
|
||||
} else if (arg_has_value(argv[i], "--early-input", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --early-input");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->early_input_file = inline_value;
|
||||
} else if (arg_has_value(argv[i], "--iconv", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
set_error(err, err_size, "missing argument for --iconv");
|
||||
return -1;
|
||||
}
|
||||
inline_value = argv[++i];
|
||||
}
|
||||
opts->iconv_spec = inline_value;
|
||||
} else if (arg_has_value(argv[i], "--dparam", &inline_value)) {
|
||||
if (!inline_value) {
|
||||
if (i + 1 >= argc) {
|
||||
@@ -260,4 +242,4 @@ void server_cli_options_free(ServerCliOptions* opts) {
|
||||
free((char**)opts->dparams);
|
||||
opts->dparams = NULL;
|
||||
opts->dparam_count = 0;
|
||||
}
|
||||
}
|
||||
|
||||
+26
-9
@@ -104,18 +104,18 @@ static void test_cli_help() {
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
/* Test that --archive's config bundle matches rsync -rlptgoD semantics:
|
||||
* links + metadata + devices + specials, and NOT compression/multithreading. */
|
||||
/* Test that the -a short spelling applies --archive's config bundle, matching
|
||||
* rsync -rlptgoD semantics: links + metadata + devices + specials, and NOT
|
||||
* compression/multithreading. (--archive itself is covered by
|
||||
* test_parse_args_archive; this guards the short alias.) */
|
||||
static void test_cli_archive_flags() {
|
||||
Config* cfg = config_create();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
char* argv[] = {"fastsync", "-a", "/src", "/dst"};
|
||||
int positional_args[2];
|
||||
int positional_count = 0;
|
||||
|
||||
/* Simulate the --archive flag's implied bundle. */
|
||||
cfg->follow_symlinks = true;
|
||||
cfg->use_metadata = true;
|
||||
cfg->preserve_devices = true;
|
||||
cfg->preserve_specials = true;
|
||||
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0);
|
||||
EXPECT_TRUE(cfg->follow_symlinks);
|
||||
EXPECT_TRUE(cfg->use_metadata);
|
||||
EXPECT_TRUE(cfg->preserve_devices);
|
||||
@@ -3045,13 +3045,30 @@ static void test_parse_args_block_size() {
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, argv_eq, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_INT((int)cfg->delta_block_size, 8192);
|
||||
|
||||
/* Out of range: parsed, warned, and the default is kept. */
|
||||
cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT;
|
||||
char* argv_delta_eq[] = {"fastsync", "--delta-block=1024", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, argv_delta_eq, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_INT((int)cfg->delta_block_size, 1024);
|
||||
|
||||
/* Out of range: parsed, warned, and the default is kept (both spellings). */
|
||||
cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT;
|
||||
char* argv_bad[] = {"fastsync", "--block-size", "1", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 5, argv_bad, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_INT((int)cfg->delta_block_size, (int)DELTA_BLOCK_SIZE_DEFAULT);
|
||||
|
||||
cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT;
|
||||
char* argv_bad_inline[] = {"fastsync", "--delta-block=999999", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, argv_bad_inline, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_INT((int)cfg->delta_block_size, (int)DELTA_BLOCK_SIZE_DEFAULT);
|
||||
|
||||
/* A non-numeric value is a hard error for both spellings. */
|
||||
char* argv_nan[] = {"fastsync", "--delta-block=abc", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, argv_nan, positional_args, &positional_count), -1);
|
||||
|
||||
/* A non-default block size changes the number of signature blocks for
|
||||
identical data: block_count = ceil(size / block_size). */
|
||||
const char data[10000] = {0};
|
||||
|
||||
+13
-6
@@ -164,19 +164,26 @@ static void test_server_cli_invalid() {
|
||||
}
|
||||
|
||||
static void test_server_cli_password_and_early_input() {
|
||||
const char* args[] = {"s", "--daemon", "--password-file=/etc/fast.pw", "--early-input",
|
||||
"/run/secrets"};
|
||||
const char* args[] = {"s",
|
||||
"--daemon",
|
||||
"--password-file=/etc/fast.pw",
|
||||
"--early-input",
|
||||
"/run/secrets",
|
||||
"--iconv=utf-8"};
|
||||
ServerCliOptions opts;
|
||||
EXPECT_EQ_INT(parse_ok(args, 5, &opts), 0);
|
||||
EXPECT_EQ_INT(parse_ok(args, 6, &opts), 0);
|
||||
EXPECT_EQ_STR(opts.password_file, "/etc/fast.pw");
|
||||
EXPECT_EQ_STR(opts.early_input_file, "/run/secrets");
|
||||
EXPECT_EQ_STR(opts.iconv_spec, "utf-8");
|
||||
|
||||
const char* args2[] = {"s", "--daemon", "--password-file", "/etc/fast.pw",
|
||||
"--early-input=/secrets"};
|
||||
const char* args2[] = {
|
||||
"s", "--daemon", "--password-file", "/etc/fast.pw", "--early-input=/secrets",
|
||||
"--iconv", "utf-8,iso-8859-1"};
|
||||
ServerCliOptions opts2;
|
||||
EXPECT_EQ_INT(parse_ok(args2, 5, &opts2), 0);
|
||||
EXPECT_EQ_INT(parse_ok(args2, 7, &opts2), 0);
|
||||
EXPECT_EQ_STR(opts2.password_file, "/etc/fast.pw");
|
||||
EXPECT_EQ_STR(opts2.early_input_file, "/secrets");
|
||||
EXPECT_EQ_STR(opts2.iconv_spec, "utf-8,iso-8859-1");
|
||||
server_cli_options_free(&opts);
|
||||
server_cli_options_free(&opts2);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user