Cleanup argument parsing and usage

[rjarry]

• main: factorize integer parsing
• main: sort help options alphabetically

Summary by CodeRabbit

• Documentation

  • Updated manual with additional options: -S, -T, and -V; reorganized option presentation for clarity.

• Bug Fixes

  • Improved error reporting with standardized messaging.
  • Enhanced validation for socket owner, group, and MTU parameters with clearer error handling.

• Refactor

  • Streamlined option parsing and help/version display flow; refined command-line argument handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes update both documentation and source code for the grout utility. The man page reorders and expands the SYNOPSIS and OPTIONS sections, adding -S, -T, and -V options. The main.c file undergoes significant refactoring: the usage/help output flow is reworked with fixed option ordering, new helper utilities are introduced (perr for error reporting to stderr and parse_uint for numeric parsing with bounds checking), option parsing logic is consolidated to use these helpers, and exit status handling is adjusted to return EXIT_SUCCESS for -h, -V, and certain parse operations.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Cleanup argument parsing and usage" directly corresponds to the stated objectives: factorizing integer parsing and sorting help options alphabetically. The changes confirm this—parse_uint helper was introduced for numeric parsing, perr was added for error reporting, and the usage/help flow was reorganized with options reordered. The title is specific enough to convey the primary changes (argument parsing improvements and usage reorganization) without being vague, and it accurately reflects the main goals of the changeset.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

main/main.c (1)

321-323: Do not unlink the socket path on early failure.

If parse_args() or dpdk_init() fails before api_socket_start(), this will unlink a running instance’s socket, breaking clients. Rely on api_socket_stop() (already called when ev_base is set) or gate unlinking on a flag set only after successful api_socket_start().

-	if (ret != EXIT_ALREADY_RUNNING)
-		unlink(gr_config.api_sock_path);
+	// Unlink is handled by api_socket_stop() when the socket was created.
+	// Avoid removing another instance's socket on early failures.

🧹 Nitpick comments (3)

main/main.c (3)

25-25: Include <stdint.h> for uint8_t.

parse_uint’s signature uses uint8_t; add the header locally for clarity/portability.

 #include <pwd.h>
-#include <stdarg.h>
+#include <stdarg.h>
+#include <stdint.h>

---

237-240: Improve unknown/missing option diagnostics for long options.

optopt may be 0 for some long-option errors; show the token instead.

-		case ':':
-			return perr("-%c requires a value", optopt);
-		case '?':
-			return perr("-%c unknown option", optopt);
+		case ':':
+			return optopt
+			       ? perr("-%c requires a value", optopt)
+			       : perr("%s requires a value", argv[optind - 1]);
+		case '?':
+			return optopt
+			       ? perr("-%c unknown option", optopt)
+			       : perr("%s unknown option", argv[optind - 1]);

---

245-247: Surface the first unexpected argument.

More actionable error message.

-	if (optind < argc)
-		return perr("invalid arguments");
+	if (optind < argc)
+		return perr("invalid argument: %s", argv[optind]);

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c4e924 and eef837e.

📒 Files selected for processing (2)

• docs/grout.8.md (1 hunks)
• main/main.c (6 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.md

⚙️ CodeRabbit configuration file

**/*.md: - Never comment about setext vs ATX style headings.

Files:

• docs/grout.8.md

**/*.{c,h}

⚙️ CodeRabbit configuration file

**/*.{c,h}: - gr_vec_*() functions cannot fail. No need to check their return value.

• gr_vec_free(x) always sets x = NULL. There is no risk of double free.
• ec_node_*() functions consume their ec_node arguments. No leaks on error.
• rte_node->ctx is an uint8_t array of size 16, not a pointer.
• Don't suggest to replace assert() with graceful error checking.
• We compile with -std=gnu2x. Unnamed parameters in function definitions are valid.

Files:

• main/main.c

🧬 Code graph analysis (1)

main/main.c (1)

api/gr_errno.h (1)

• errno_set (9-12)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

• GitHub Check: deb
• GitHub Check: rpm
• GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
• GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
• GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
• GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
• GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
• GitHub Check: build-cross-aarch64

🔇 Additional comments (4)

docs/grout.8.md (1)

19-22: SYNOPSIS reordering looks good; matches main’s usage.

No functional issues spotted. Keep docs and binary help in lockstep; we’re fixing a small “REGEXO” typo in main’s help separately.

main/main.c (3)

78-90: perr(): simple, consistent error reporting.

LGTM.

---

91-108: parse_uint(): robust bounds/format checks.

LGTM. With the <stdint.h> include added, this is solid.

---

116-137: socket owner parsing: clear UX and sane fallbacks.

Nice: name→ID first, then numeric with precise perr messages. No change needed.

[coderabbitai]

⚠️ 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.

[aharivel]

Both commits looks good to me !