Skip to content

Cleanup argument parsing and usage #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christophefontaine merged 2 commits into from
Oct 23, 2025
+88 −103
Merged

Cleanup argument parsing and usage #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66a7706
main: factorize integer parsing
rjarry Oct 22, 2025
eef837e
main: sort help options alphabetically
rjarry Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/grout.8.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ Grout is a software router based on DPDK __rte_graph__.
**grout**
[**-B** _SIZE_]
[**-D** _PATH_]
[**-h**]
[**-L** _TYPE_:_LEVEL_]
[**-M** _MODE_]
[**-S**]
[**-T** _REGEXP_]
[**-V**]
[**-h**]
[**-m** _PERMISSIONS_]
[**-o** _USER_:_GROUP_]
[**-p**]
[**-S**]
[**-s** _PATH_]
[**-t**]
[**-T** _REGEXP_]
[**-u** _MTU_]
[**-v**]
[**-V**]
[**-x**]

# OPTIONS
Expand Down
183 changes: 84 additions & 99 deletions main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <grp.h>
#include <locale.h>
#include <pwd.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
Expand All @@ -30,127 +31,116 @@

// Please keep options/flags in alphabetical order.

static void usage(const char *prog) {
printf("Usage: %s [-h] [-L <type>:<lvl>] [-p] [-s <path>] [-t] [-u <mtu>]\n", prog);
printf(" %*s [-T <regexp>] [-B <size>] [-D <path>] [-M <mode>] [-x] [-v] [-V]\n",
(int)strlen(prog),
"");
static void usage(void) {
printf("Usage: grout");
printf(" [-B SIZE]");
printf(" [-D PATH]");
printf(" [-L TYPE:LEVEL]");
printf(" [-M MODE]");
printf(" [-S]");
printf(" [-T REGEXP]");
printf(" [-V]");
printf(" [-h]");
printf("\n ");
printf(" [-m PERMISSIONS]");
printf(" [-o USER:GROUP]");
printf(" [-p]");
printf(" [-s PATH]");
printf(" [-t]");
printf(" [-u MTU]");
printf(" [-v]");
printf(" [-x]");
puts("");
puts("");
printf(" Graph router version %s (%s).\n", GROUT_VERSION, rte_version());
puts("");
puts("options:");
puts(" -B, --trace-bufsz SIZE Maximum size of allocated memory for trace output.");
puts(" -D, --trace-dir PATH Change path for trace output.");
puts(" -L, --log-level TYPE:LEVEL Specify log level for a specific component.");
puts(" -M, --trace-mode MODE Specify the mode of update of trace output file.");
puts(" -S, --syslog Redirect logs to syslog.");
puts(" -T, --trace REGEXO Enable trace matching the regular expression.");
puts(" -V, --version Print version and exit.");
puts(" -h, --help Display this help message and exit.");
puts(" -L, --log-level <type>:<lvl> Specify log level for a specific component.");
puts(" -m, --socket-mode <mode> API socket file permissions (Default: 0660).");
puts(" -o, --socket-owner <user>:<group> API socket file ownership");
puts(" (Default: getuid():getgid()).");
puts(" -m, --socket-mode PERMISSIONS API socket file permissions (Default: 0660).");
puts(" -o, --socket-owner USER:GROUP API socket file ownership");
puts(" -p, --poll-mode Disable automatic micro-sleep.");
puts(" -S, --syslog Redirect logs to syslog.");
puts(" -s, --socket <path> Path the control plane API socket.");
puts(" -s, --socket PATH Path the control plane API socket.");
puts(" Default: GROUT_SOCK_PATH from env or");
printf(" %s).\n", GR_DEFAULT_SOCK_PATH);
puts(" -t, --test-mode Run in test mode (no hugepages).");
puts(" -u, --max-mtu <mtu> Maximum Transmission Unit (default 1800).");
puts(" -T, --trace <regexp> Enable trace matching the regular expression.");
puts(" -B, --trace-bufsz <size> Maximum size of allocated memory for trace output.");
puts(" -D, --trace-dir <path> Change path for trace output.");
puts(" -M, --trace-mode <mode> Specify the mode of update of trace output file.");
puts(" -x, --trace-packets Print all ingress/egress packets.");
puts(" -u, --max-mtu MTU Maximum Transmission Unit (default 1800).");
puts(" -v, --verbose Increase verbosity.");
puts(" -V, --version Print version and exit.");
puts(" -x, --trace-packets Print all ingress/egress packets.");
}
Comment on lines +34 to +76
Copy link

@coderabbitai coderabbitai bot Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Help text nits: fix “REGEXO”, grammar, and stray ‘)’.

Tighten the usage output.

-	puts("  -T, --trace REGEXO             Enable trace matching the regular expression.");
+	puts("  -T, --trace REGEXP             Enable trace matching the regular expression.");
-	puts("  -s, --socket PATH              Path the control plane API socket.");
+	puts("  -s, --socket PATH              Path to the control plane API socket.");
-	puts("                                 Default: GROUT_SOCK_PATH from env or");
-	printf("                                 %s).\n", GR_DEFAULT_SOCK_PATH);
+	puts("                                 Default: GROUT_SOCK_PATH from env or");
+	printf("                                 %s.\n", GR_DEFAULT_SOCK_PATH);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static void usage(void) {
printf("Usage: grout");
printf(" [-B SIZE]");
printf(" [-D PATH]");
printf(" [-L TYPE:LEVEL]");
printf(" [-M MODE]");
printf(" [-S]");
printf(" [-T REGEXP]");
printf(" [-V]");
printf(" [-h]");
printf("\n ");
printf(" [-m PERMISSIONS]");
printf(" [-o USER:GROUP]");
printf(" [-p]");
printf(" [-s PATH]");
printf(" [-t]");
printf(" [-u MTU]");
printf(" [-v]");
printf(" [-x]");
puts("");
puts("");
printf(" Graph router version %s (%s).\n", GROUT_VERSION, rte_version());
puts("");
puts("options:");
puts(" -B, --trace-bufsz SIZE Maximum size of allocated memory for trace output.");
puts(" -D, --trace-dir PATH Change path for trace output.");
puts(" -L, --log-level TYPE:LEVEL Specify log level for a specific component.");
puts(" -M, --trace-mode MODE Specify the mode of update of trace output file.");
puts(" -S, --syslog Redirect logs to syslog.");
puts(" -T, --trace REGEXO Enable trace matching the regular expression.");
puts(" -V, --version Print version and exit.");
puts(" -h, --help Display this help message and exit.");
puts(" -L, --log-level <type>:<lvl> Specify log level for a specific component.");
puts(" -m, --socket-mode <mode> API socket file permissions (Default: 0660).");
puts(" -o, --socket-owner <user>:<group> API socket file ownership");
puts(" (Default: getuid():getgid()).");
puts(" -m, --socket-mode PERMISSIONS API socket file permissions (Default: 0660).");
puts(" -o, --socket-owner USER:GROUP API socket file ownership");
puts(" -p, --poll-mode Disable automatic micro-sleep.");
puts(" -S, --syslog Redirect logs to syslog.");
puts(" -s, --socket <path> Path the control plane API socket.");
puts(" -s, --socket PATH Path the control plane API socket.");
puts(" Default: GROUT_SOCK_PATH from env or");
printf(" %s).\n", GR_DEFAULT_SOCK_PATH);
puts(" -t, --test-mode Run in test mode (no hugepages).");
puts(" -u, --max-mtu <mtu> Maximum Transmission Unit (default 1800).");
puts(" -T, --trace <regexp> Enable trace matching the regular expression.");
puts(" -B, --trace-bufsz <size> Maximum size of allocated memory for trace output.");
puts(" -D, --trace-dir <path> Change path for trace output.");
puts(" -M, --trace-mode <mode> Specify the mode of update of trace output file.");
puts(" -x, --trace-packets Print all ingress/egress packets.");
puts(" -u, --max-mtu MTU Maximum Transmission Unit (default 1800).");
puts(" -v, --verbose Increase verbosity.");
puts(" -V, --version Print version and exit.");
puts(" -x, --trace-packets Print all ingress/egress packets.");
}
static void usage(void) {
printf("Usage: grout");
printf(" [-B SIZE]");
printf(" [-D PATH]");
printf(" [-L TYPE:LEVEL]");
printf(" [-M MODE]");
printf(" [-S]");
printf(" [-T REGEXP]");
printf(" [-V]");
printf(" [-h]");
printf("\n ");
printf(" [-m PERMISSIONS]");
printf(" [-o USER:GROUP]");
printf(" [-p]");
printf(" [-s PATH]");
printf(" [-t]");
printf(" [-u MTU]");
printf(" [-v]");
printf(" [-x]");
puts("");
puts("");
printf(" Graph router version %s (%s).\n", GROUT_VERSION, rte_version());
puts("");
puts("options:");
puts(" -B, --trace-bufsz SIZE Maximum size of allocated memory for trace output.");
puts(" -D, --trace-dir PATH Change path for trace output.");
puts(" -L, --log-level TYPE:LEVEL Specify log level for a specific component.");
puts(" -M, --trace-mode MODE Specify the mode of update of trace output file.");
puts(" -S, --syslog Redirect logs to syslog.");
puts(" -T, --trace REGEXP Enable trace matching the regular expression.");
puts(" -V, --version Print version and exit.");
puts(" -h, --help Display this help message and exit.");
puts(" -m, --socket-mode PERMISSIONS API socket file permissions (Default: 0660).");
puts(" -o, --socket-owner USER:GROUP API socket file ownership");
puts(" -p, --poll-mode Disable automatic micro-sleep.");
puts(" -s, --socket PATH Path to the control plane API socket.");
puts(" Default: GROUT_SOCK_PATH from env or");
printf(" %s.\n", GR_DEFAULT_SOCK_PATH);
puts(" -t, --test-mode Run in test mode (no hugepages).");
puts(" -u, --max-mtu MTU Maximum Transmission Unit (default 1800).");
puts(" -v, --verbose Increase verbosity.");
puts(" -x, --trace-packets Print all ingress/egress packets.");
}
🤖 Prompt for AI Agents 
In main/main.c around lines 34 to 76, tighten and correct the usage/help output:
change the typo "REGEXO" to "REGEXP"; fix the grammar "Path the control plane
API socket." → "Path to the control plane API socket."; remove the stray closing
parenthesis before the newline in the default socket path printf (currently
"%s).\n" → "%s\n" or move the closing parenthesis into the format string
correctly); and remove redundant blank puts/printf calls (the double blank lines
and the awkward printf("\n            ");) to produce a cleaner, condensed help
output.


static int perr(const char *fmt, ...) {
char buf[512];
va_list ap;

va_start(ap, fmt);
vsnprintf(buf, sizeof(buf), fmt, ap);
va_end(ap);

fprintf(stderr, "error: %s\n", buf);

return -1;
}

static int
parse_uint(unsigned int *v, const char *s, uint8_t base, unsigned long min, unsigned long max) {
unsigned long val;
char *endptr;

errno = 0;
val = strtoul(s, &endptr, base);
if (errno != 0)
return errno_set(errno);
if (*endptr != '\0')
return errno_set(EINVAL);
if (val < min || val > max)
return errno_set(ERANGE);

*v = val;

return 0;
}

struct gr_config gr_config;

static int parse_sock_owner(char *user_group_str) {
char *group_str, *user_str = user_group_str;
struct passwd *pw;
char *colon, *end;
unsigned long val;
struct group *gr;
char *colon;

colon = strchr(user_group_str, ':');
if (!colon) {
fprintf(stderr, "error: -%c requires ':'\n", optopt);
return -1;
}
if (!colon)
return perr("--socket-owner: missing ':'");

*colon = '\0';
group_str = colon + 1;

pw = getpwnam(user_str);
if (!pw) {
errno = 0;
val = strtoul(user_str, &end, 10);

if (errno || *end != '\0' || val > (uid_t)-1) {
fprintf(stderr, "error: invalid user '%s'\n", user_str);
return -1;
}

gr_config.api_sock_uid = (uid_t)val;
if (parse_uint(&gr_config.api_sock_uid, user_str, 10, 0, (uid_t)-1) < 0)
return perr("--socket-owner: <user>: %s", strerror(errno));
} else {
gr_config.api_sock_uid = pw->pw_uid;
}

gr = getgrnam(group_str);
if (!gr) {
errno = 0;
val = strtoul(group_str, &end, 10);

if (errno || *end != '\0' || val > (gid_t)-1) {
fprintf(stderr, "error: invalid group %s\n", group_str);
return -1;
}

gr_config.api_sock_gid = (gid_t)val;
if (parse_uint(&gr_config.api_sock_gid, group_str, 10, 0, (gid_t)-1) < 0)
return perr("--socket-owner: <group>: %s", strerror(errno));
} else {
gr_config.api_sock_gid = gr->gr_gid;
}

return 0;
}

static int parse_sock_mode(const char *perm_str) {
unsigned long val;
char *endptr;

errno = 0;
val = strtoul(perm_str, &endptr, 8);

if (errno != 0 || *endptr != '\0' || val > 07777) {
fprintf(stderr, "error: invalid permissions '%s'\n", perm_str);
return -1;
}

gr_config.api_sock_mode = (mode_t)val;

return 0;
}

static int parse_max_mtu(const char *mtu) {
unsigned long val;
char *endptr;

errno = 0;
val = strtoul(mtu, &endptr, 10);

if (errno != 0 || *endptr != '\0' || val < 512 || val > 16384) {
if (errno == 0) {
if (*endptr != '\0')
errno = EINVAL;
else
errno = ERANGE;
}
fprintf(stderr, "error: invalid max-mtu '%s': %s\n", mtu, strerror(errno));
return -1;
}

gr_config.max_mtu = (unsigned)val;

return 0;
}

static int parse_args(int argc, char **argv) {
int c;

Expand All @@ -160,10 +150,10 @@ static int parse_args(int argc, char **argv) {
{"log-level", required_argument, NULL, 'L'},
{"max-mtu", required_argument, NULL, 'u'},
{"poll-mode", no_argument, NULL, 'p'},
{"syslog", no_argument, NULL, 'S'},
{"socket", required_argument, NULL, 's'},
{"socket-mode", required_argument, NULL, 'm'},
{"socket-owner", required_argument, NULL, 'o'},
{"syslog", no_argument, NULL, 'S'},
{"test-mode", no_argument, NULL, 't'},
{"trace", required_argument, NULL, 'T'},
{"trace-bufsz", required_argument, NULL, 'B'},
Expand All @@ -190,15 +180,16 @@ static int parse_args(int argc, char **argv) {
while ((c = getopt_long(argc, argv, FLAGS, long_options, NULL)) != -1) {
switch (c) {
case 'h':
usage(argv[0]);
return -1;
usage();
exit(EXIT_SUCCESS);
break;
case 'L':
gr_vec_add(gr_config.eal_extra_args, "--log-level");
gr_vec_add(gr_config.eal_extra_args, optarg);
break;
case 'm':
if (parse_sock_mode(optarg) < 0)
return errno_set(EINVAL);
if (parse_uint(&gr_config.api_sock_mode, optarg, 8, 0, 07777) < 0)
return perr("--socket-mode: %s", strerror(errno));
break;
case 'o':
if (parse_sock_owner(optarg) < 0)
Expand Down Expand Up @@ -232,8 +223,8 @@ static int parse_args(int argc, char **argv) {
gr_config.log_packets = true;
break;
case 'u':
if (parse_max_mtu(optarg) < 0)
return errno_set(EINVAL);
if (parse_uint(&gr_config.max_mtu, optarg, 10, 512, 16384) < 0)
return perr("--max-mtu: %s", strerror(errno));
break;
case 'v':
gr_config.log_level++;
Expand All @@ -243,22 +234,16 @@ static int parse_args(int argc, char **argv) {
exit(EXIT_SUCCESS);
break;
case ':':
usage(argv[0]);
fprintf(stderr, "error: -%c requires a value\n", optopt);
return errno_set(EINVAL);
return perr("-%c requires a value", optopt);
case '?':
usage(argv[0]);
fprintf(stderr, "error: -%c unknown option\n", optopt);
return errno_set(EINVAL);
return perr("-%c unknown option", optopt);
default:
goto end;
}
}
end:
if (optind < argc) {
fputs("error: invalid arguments", stderr);
return errno_set(EINVAL);
}
if (optind < argc)
return perr("invalid arguments");

return 0;
}
Expand Down