diff mbox series

[iproute2,v2] ip: drop 2-char command assumption

Message ID 20210420082636.1210305-1-Tony.Ambardar@gmail.com (mailing list archive)
State Accepted
Delegated to: David Ahern
Headers show
Series [iproute2,v2] ip: drop 2-char command assumption | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Tony Ambardar April 20, 2021, 8:26 a.m. UTC
The 'ip' utility hardcodes the assumption of being a 2-char command, where
any follow-on characters are passed as an argument:

  $ ./ip-full help
  Object "-full" is unknown, try "ip help".

This confusing behaviour isn't seen with 'tc' for example, and was added in
a 2005 commit without documentation. It was noticed during testing of 'ip'
variants built/packaged with different feature sets (e.g. w/o BPF support).

Mitigate the problem by redoing the command without the 2-char assumption
if the follow-on characters fail to parse as a valid command.

Fixes: 351efcde4e62 ("Update header files to 2.6.14")
Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
---
v2: (feedback from David Ahern)
  * work around problem but remain compatible with 2-char assumption

---
 ip/ip.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Stephen Hemminger April 20, 2021, 3:16 p.m. UTC | #1
On Tue, 20 Apr 2021 01:26:36 -0700
Tony Ambardar <tony.ambardar@gmail.com> wrote:

> The 'ip' utility hardcodes the assumption of being a 2-char command, where
> any follow-on characters are passed as an argument:
> 
>   $ ./ip-full help
>   Object "-full" is unknown, try "ip help".
> 
> This confusing behaviour isn't seen with 'tc' for example, and was added in
> a 2005 commit without documentation. It was noticed during testing of 'ip'
> variants built/packaged with different feature sets (e.g. w/o BPF support).
> 
> Mitigate the problem by redoing the command without the 2-char assumption
> if the follow-on characters fail to parse as a valid command.
> 
> Fixes: 351efcde4e62 ("Update header files to 2.6.14")
> Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
> ---
> v2: (feedback from David Ahern)
>   * work around problem but remain compatible with 2-char assumption

I am ok with this, but if you change the name of command, you can expect some
friction (and non support).

The original commit was inherited from the original integration of tarball's
into BitKeeper. This "feature" was put in by Alexey Kuznetsov back in orignal 2.4
time frame.
Tony Ambardar April 21, 2021, 4:49 a.m. UTC | #2
On Tue, 20 Apr 2021 at 08:16, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 20 Apr 2021 01:26:36 -0700
> Tony Ambardar <tony.ambardar@gmail.com> wrote:
>
> > The 'ip' utility hardcodes the assumption of being a 2-char command, where
> > any follow-on characters are passed as an argument:
> >
> >   $ ./ip-full help
> >   Object "-full" is unknown, try "ip help".
> >
> > This confusing behaviour isn't seen with 'tc' for example, and was added in
> > a 2005 commit without documentation. It was noticed during testing of 'ip'
> > variants built/packaged with different feature sets (e.g. w/o BPF support).
> >
> > Mitigate the problem by redoing the command without the 2-char assumption
> > if the follow-on characters fail to parse as a valid command.
> >
> > Fixes: 351efcde4e62 ("Update header files to 2.6.14")
> > Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
> > ---
> > v2: (feedback from David Ahern)
> >   * work around problem but remain compatible with 2-char assumption
>
> I am ok with this, but if you change the name of command, you can expect some
> friction (and non support).
>

The renamed binaries are normally accessed via an 'ip' symlink managed
at package installation, so shouldn't be an issue. I only discovered
the problem while running regression tests on the the underlying
binaries, and thought to fix things.

> The original commit was inherited from the original integration of tarball's
> into BitKeeper. This "feature" was put in by Alexey Kuznetsov back in orignal 2.4
> time frame.

Thanks for the feedback and additional detail,
Tony
David Ahern April 26, 2021, 2:31 a.m. UTC | #3
On 4/20/21 1:26 AM, Tony Ambardar wrote:
> The 'ip' utility hardcodes the assumption of being a 2-char command, where
> any follow-on characters are passed as an argument:
> 
>   $ ./ip-full help
>   Object "-full" is unknown, try "ip help".
> 
> This confusing behaviour isn't seen with 'tc' for example, and was added in
> a 2005 commit without documentation. It was noticed during testing of 'ip'
> variants built/packaged with different feature sets (e.g. w/o BPF support).
> 
> Mitigate the problem by redoing the command without the 2-char assumption
> if the follow-on characters fail to parse as a valid command.
> 
> Fixes: 351efcde4e62 ("Update header files to 2.6.14")
> Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
> ---
> v2: (feedback from David Ahern)
>   * work around problem but remain compatible with 2-char assumption
> 
> ---
>  ip/ip.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 

Applied. Thanks,
diff mbox series

Patch

diff --git a/ip/ip.c b/ip/ip.c
index 4cf09fc3..8e4c6eb5 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -125,7 +125,7 @@  static const struct cmd {
 	{ 0 }
 };
 
-static int do_cmd(const char *argv0, int argc, char **argv)
+static int do_cmd(const char *argv0, int argc, char **argv, bool final)
 {
 	const struct cmd *c;
 
@@ -134,7 +134,8 @@  static int do_cmd(const char *argv0, int argc, char **argv)
 			return -(c->func(argc-1, argv+1));
 	}
 
-	fprintf(stderr, "Object \"%s\" is unknown, try \"ip help\".\n", argv0);
+	if (final)
+		fprintf(stderr, "Object \"%s\" is unknown, try \"ip help\".\n", argv0);
 	return EXIT_FAILURE;
 }
 
@@ -143,7 +144,7 @@  static int ip_batch_cmd(int argc, char *argv[], void *data)
 	const int *orig_family = data;
 
 	preferred_family = *orig_family;
-	return do_cmd(argv[0], argc, argv);
+	return do_cmd(argv[0], argc, argv, true);
 }
 
 static int batch(const char *name)
@@ -313,11 +314,14 @@  int main(int argc, char **argv)
 
 	rtnl_set_strict_dump(&rth);
 
-	if (strlen(basename) > 2)
-		return do_cmd(basename+2, argc, argv);
+	if (strlen(basename) > 2) {
+		int ret = do_cmd(basename+2, argc, argv, false);
+		if (ret != EXIT_FAILURE)
+			return ret;
+	}
 
 	if (argc > 1)
-		return do_cmd(argv[1], argc-1, argv+1);
+		return do_cmd(argv[1], argc-1, argv+1, true);
 
 	rtnl_close(&rth);
 	usage();