From: Petr Machata <petrm@nvidia.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Petr Machata <petrm@nvidia.com>, David Ahern <dsahern@gmail.com>,
<netdev@vger.kernel.org>, Patrisious Haddad <phaddad@nvidia.com>
Subject: Re: [PATCH iproute2-next 3/3] lib: utils: Add parse_one_of_deprecated(), parse_on_off_deprecated()
Date: Wed, 15 Nov 2023 17:57:27 +0100 [thread overview]
Message-ID: <87pm0bvswu.fsf@nvidia.com> (raw)
In-Reply-To: <20231115075921.198fad24@hermes.local>
Stephen Hemminger <stephen@networkplumber.org> writes:
> On Wed, 15 Nov 2023 16:31:59 +0100
> Petr Machata <petrm@nvidia.com> wrote:
>
>> The functions parse_on_off() and parse_one_of() currently use 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. By sheer luck, the end result actually
>> makes some sense: "on" means on, anything else means off (or errors out).
>> Similar issues are in principle also possible for parse_one_of() uses,
>> though currently this does not come up.
>
> This was probably a bug, I am open to breaking shorthand usage in this case.
There were uses of matches() for on/off parsing even before adding
parse_on_off(). The bug was converting _everyone_ to matches().
I figured you'd be against just s/matches/strcmp, but if you think it's
OK, I have no problem with that. Shorthanding on/off just makes no
sense to me, not even by mistake.
How about the parse_one_of() users? E.g. the disabled/check/strict for
macsec validate, I could see someone mistyping it as "disable", so now
it lives in some deployment script, or testing harness, or whatever.
Maybe do the warning thing in this case? And retire it a couple releases
down the line in favor of just accepting strcmp?
next prev parent reply other threads:[~2023-11-15 17:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-15 15:31 [PATCH iproute2-next 0/3] Change parse_one_of(), parse_on_off() to strcmp() Petr Machata
2023-11-15 15:31 ` [PATCH iproute2-next 1/3] lib: utils: Switch matches() to returning int again Petr Machata
2023-11-15 15:31 ` [PATCH iproute2-next 2/3] lib: utils: Generalize code of parse_one_of(), parse_on_off() Petr Machata
2023-11-15 15:31 ` [PATCH iproute2-next 3/3] lib: utils: Add parse_one_of_deprecated(), parse_on_off_deprecated() Petr Machata
2023-11-15 15:57 ` Stephen Hemminger
2023-11-15 15:59 ` Stephen Hemminger
2023-11-15 16:57 ` Petr Machata [this message]
2023-11-20 22:27 ` David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pm0bvswu.fsf@nvidia.com \
--to=petrm@nvidia.com \
--cc=dsahern@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=phaddad@nvidia.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).