[iproute2-next,v2,3/5] lib: utils: Convert parse_on_off() to strcmp()

Commit Message

Petr Machata Nov. 22, 2023, 3:23 p.m. UTC

The function parse_on_off() currently uses matches() for string comparison
under the hood. This has some odd consequences. In particular, "o" can be
used as a shorthand for "off", which is not obvious, because "o" is the
prefix of both. In this patch, change parsing to strcmp(). This is a
breaking change. The following paragraphs give arguments for why it should
be considered acceptable.

First and foremost: on/off are very short strings that it makes practically
no sense to shorten. Since "o" is the universal prefix, the only
unambiguous shortening is "of" for "off". It is doubtful that anyone would
intentionally decide to save typing of the second "f" when they already
typed the first. It also seems unlikely that the typo of "of" for "off"
would not be caught immediately, as missing a third of the word length
would likely be noticed. In other words, it seems improbable that the
abbreviated variants are used, intentionally or by mistake.

Commit 9262ccc3ed32 ("bridge: link: Port over to parse_on_off()") and
commit 3e0d2a73ba06 ("ip: iplink_bridge_slave: Port over to
parse_on_off()") converted several sites from open-coding strcmp()-based
on/off parsing to parse_on_off(), which is itself based on matches(). This
made the list of permissible strings more generic, but the behavior was
exact match to begin with, and this patch restores it.

Commit 5f685d064b03 ("ip: iplink: Convert to use parse_on_off()") has
changed from matches()-based parsing, which however had branches in the
other order, and "o" would parse to mean on. This indicates that at least
in this context, people were not using the shorthand of "o" or the commit
would have broken their use case. This supports the thesis that the
abbreviations are not really used for on/off parsing.

For completeness, commit 82604d28525a ("lib: Add parse_one_of(),
parse_on_off()") introduced parse_on_off(), converting several users in the
ip link macsec code in the process. Those users have always used matches(),
and had branches in the same order as the newly-introduced parse_on_off().

A survey of selftests and documentation of Linux kernel (by way of git
grep), has not discovered any cases of the involved options getting
arguments other than the exact strings on and off.

Signed-off-by: Petr Machata <[e-mail]>
---
 lib/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch

diff --git a/lib/utils.c b/lib/utils.c
index 5c91aaa9..f1ca3852 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1763,7 +1763,7 @@  bool parse_on_off(const char *msg, const char *realval, int *p_err)
 	static const char * const values_on_off[] = { "off", "on" };
 
 	return __parse_one_of(msg, realval, values_on_off,
-			      ARRAY_SIZE(values_on_off), p_err, matches);
+			      ARRAY_SIZE(values_on_off), p_err, strcmp);
 }
 
 int parse_mapping_gen(int *argcp, char ***argvp,