[PATCH] ip: Make 'ip stats help' exit with -1

[PATCH] ip: Make 'ip stats help' exit with -1

[Yedaya Katsman]

ip <subcommand> help exits with -1 for all commands except for stats, make
it match.
This removes a goto err, which is redundant since "enabled" gets
allocated only if we find a level, but we enter do_help only if we didn't.

Fixes: df0b2c6d0098 ("ipstats: Add a shell of "show" command")
Signed-off-by: Yedaya Katsman <yedaya.ka@gmail.com>
---
 ip/ipstats.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index f0f8dcdcbd2a9e1c83eed6ca54df28b91c058d9c..812a11dacb2f5851ea5d16644b82e8a7a0428e5a 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -1085,7 +1085,9 @@ static int ipstats_enable_check(struct ipstats_sel *sel,
 	return err;
 }
 
-static int do_help(void)
+static void do_help(void) __attribute__((noreturn));
+
+static void do_help(void)
 {
 	const struct ipstats_stat_desc *toplev = &ipstats_stat_desc_toplev_group;
 	int i;
@@ -1135,8 +1137,7 @@ static int do_help(void)
 		if (opened)
 			fprintf(stderr, " }\n");
 	}
-
-	return 0;
+	exit(-1);
 }
 
 static int ipstats_select(struct ipstats_sel *old_sel,
@@ -1190,7 +1191,6 @@ static int ipstats_show(int argc, char **argv)
 			dev = *argv;
 		} else if (strcmp(*argv, "help") == 0) {
 			do_help();
-			return 0;
 		} else {
 			bool found_level = false;
 
@@ -1209,8 +1209,6 @@ static int ipstats_show(int argc, char **argv)
 			if (!found_level) {
 				fprintf(stderr, "What is \"%s\"?\n", *argv);
 				do_help();
-				err = -EINVAL;
-				goto err;
 			}
 		}
 
@@ -1286,11 +1284,9 @@ static int ipstats_set(int argc, char **argv)
 				return err;
 		} else if (strcmp(*argv, "help") == 0) {
 			do_help();
-			return 0;
 		} else {
 			fprintf(stderr, "What is \"%s\"?\n", *argv);
 			do_help();
-			return -EINVAL;
 		}
 
 		NEXT_ARG_FWD();
@@ -1321,7 +1317,6 @@ int do_ipstats(int argc, char **argv)
 		rc = ipstats_show(0, NULL);
 	} else if (strcmp(*argv, "help") == 0) {
 		do_help();
-		rc = 0;
 	} else if (strcmp(*argv, "show") == 0) {
 		/* Invoking "stats show" implies one -s. Passing -d adds one
 		 * more -s.

---
base-commit: 218757a8b8bb431b382d7a104edb6ed4336d3b43
change-id: 20260301-ip-stats-exit-code-ce417579bcb6

Best regards,
-- 
Yedaya Katsman <yedaya.ka@gmail.com>

Re: [PATCH] ip: Make 'ip stats help' exit with -1

[Stephen Hemminger]

On Sun, 01 Mar 2026 16:12:03 +0200
Yedaya Katsman <yedaya.ka@gmail.com> wrote:

> ip <subcommand> help exits with -1 for all commands except for stats, make
> it match.
> This removes a goto err, which is redundant since "enabled" gets
> allocated only if we find a level, but we enter do_help only if we didn't.
> 
> Fixes: df0b2c6d0098 ("ipstats: Add a shell of "show" command")
> Signed-off-by: Yedaya Katsman <yedaya.ka@gmail.com>

You raise a good point, but not sure that fixing just one corner is the
right answer here. Most of iproute2 gets this wrong already.

The (poorly documented) convention for help in Unix commands:
Output destination: stdout (not stderr). 
Help text is expected, requested output — not an error.
This matters for piping, e.g. command --help | grep something.

Exit code: 0 (success).
The user asked for help and got it — that's a successful operation.

Contrast with error cases:
When a user provides an invalid flag or misuses the command, 
the usage/error message should go to stderr with a non-zero exit code
(typically 1 or 2 — 2 is common convention from many GNU tools for usage errors).

In practice, this is codified in the GNU Coding Standards, 
which explicitly state that --help should print to stdout and exit with status 0.
POSIX doesn't mandate -h specifically, but the GNU convention is widely followed and expected.
A surprising number of tools historically got this wrong (sending help to stderr or returning non-zero).

But iproute2 does not follow this convention.
Fixing it might be a good idea, the kind of thing AI refactoring might help at.

An example which should print nothing!
$ ip help >/dev/null || echo broken 
Usage: ip [ OPTIONS ] OBJECT { COMMAND | help }
       ip [ -force ] -batch filename
where  OBJECT := { address | addrlabel | fou | help | ila | ioam | l2tp | link |
                   macsec | maddress | monitor | mptcp | mroute | mrule |
                   neighbor | neighbour | netconf | netns | nexthop | ntable |
                   ntbl | route | rule | sr | stats | tap | tcpmetrics |
                   token | tunnel | tuntap | vrf | xfrm }
       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |
                    -h[uman-readable] | -iec | -j[son] | -p[retty] |
                    -f[amily] { inet | inet6 | mpls | bridge | link } |
                    -4 | -6 | -M | -B | -0 |
                    -l[oops] { maximum-addr-flush-attempts } | -echo | -br[ief] |
                    -o[neline] | -t[imestamp] | -ts[hort] | -b[atch] [filename] |
                    -rc[vbuf] [size] | -n[etns] name | -N[umeric] | -a[ll] |
                    -c[olor]}
broken