| Message ID | cover.1700666420.git.petrm@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Change parsing in parse_one_of(), parse_on_off() | expand |
Hello: This series was applied to iproute2/iproute2-next.git (main) by David Ahern <dsahern@kernel.org>: On Wed, 22 Nov 2023 16:23:27 +0100 you wrote: > Library functions parse_one_of() and parse_on_off() were added about three > years ago to unify all the disparate reimplementations of the same basic > idea. It used the matches() function to determine whether a string under > consideration corresponds to one of the keywords. This reflected many, > though not all cases of on/off parsing at the time. > > This decision 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. By sheer luck, the end result actually makes some sense: "on" means > on, anything else either means off or errors out. Similar issues are in > principle also possible for parse_one_of() uses, though currently this does > not come up. > > [...] Here is the summary with links: - [iproute2-next,v2,1/5] lib: utils: Switch matches() to returning int again https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=60254925ccab - [iproute2-next,v2,2/5] lib: utils: Generalize parse_one_of() https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=256e0ca4b84f - [iproute2-next,v2,3/5] lib: utils: Convert parse_on_off() to strcmp() https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=5ba57152d27c - [iproute2-next,v2,4/5] lib: utils: Introduce parse_one_of_deprecated() https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=2b8766663d3c - [iproute2-next,v2,5/5] lib: utils: Have parse_one_of() warn about prefix matches https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=bd5226437a4c You are awesome, thank you!
Library functions parse_one_of() and parse_on_off() were added about three years ago to unify all the disparate reimplementations of the same basic idea. It used the matches() function to determine whether a string under consideration corresponds to one of the keywords. This reflected many, though not all cases of on/off parsing at the time. This decision 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. By sheer luck, the end result actually makes some sense: "on" means on, anything else either means off or errors out. Similar issues are in principle also possible for parse_one_of() uses, though currently this does not come up. Ideally parse_on_off() would accept the strings "on" and "off" and no others. Patch #1 is a cleanup. Patch #2 is shaping the code for the next patches. Patch #3 converts parse_on_off() to strcmp(). See the commit message for the rationale of why the change should be considered acceptable. We'd ideally do parse_one_of() likewise. But the strings this function parses tend to be longer, which means more opportunities for typos and more of a reason to abbreviate things. So instead, patch #4 adds a function parse_one_of_deprecated() for ip macsec to use in one place, where these typos are to be expected, and converts that site to the new function. Then patch #5 changes the behavior of parse_one_of() to accept prefixes like it has so far, but to warn that they are deprecated: # dcb ets set dev swp1 tc-tsa 0:s WARNING: 's' matches 'strict' by prefix. Matching by prefix is deprecated in this context, please use the full string. The idea is that several releases down the line, we might consider switching over to strcmp(), as presumably enough advance warning will have been given. v2: - Ditch parse_on_off_deprecated() and just use strcmp outright. - Only use parse_one_of_deprecated() for one call site. Change parse_one_of() to warn on prefix matches. Petr Machata (5): lib: utils: Switch matches() to returning int again lib: utils: Generalize parse_one_of() lib: utils: Convert parse_on_off() to strcmp() lib: utils: Introduce parse_one_of_deprecated() lib: utils: Have parse_one_of() warn about prefix matches include/utils.h | 5 ++++- ip/ipmacsec.c | 6 ++++-- lib/utils.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 49 insertions(+), 11 deletions(-)