netdev - Re: [PATCH iproute2-next v2 03/11] lib: utils: Add print_on_off

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874km6i28j.fsf@nvidia.com>
Date:   Tue, 03 Nov 2020 22:01:32 +0100
From:   Petr Machata <me@...chata.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
        stephen@...workplumber.org, john.fastabend@...il.com,
        jiri@...dia.com, idosch@...dia.com,
        Jakub Kicinski <kuba@...nel.org>,
        Roman Mashak <mrv@...atatu.com>
Subject: Re: [PATCH iproute2-next v2 03/11] lib: utils: Add print_on_off_bool()


Leon Romanovsky <leon@...nel.org> writes:

> On Tue, Nov 03, 2020 at 12:05:20AM +0100, Petr Machata wrote:
>>
>> Leon Romanovsky <leon@...nel.org> writes:
>>
>> > On Sun, Nov 01, 2020 at 04:55:42PM -0700, David Ahern wrote:
>> >
>> >> yes, the rdma utils are using generic function names. The rdma version
>> >> should be renamed; perhaps rd_print_on_off. That seems to be once common
>> >> prefix. Added Leon.
>> >
>> > I made fast experiment and the output for the code proposed here and existed
>> > in the RDMAtool - result the same. So the good thing will be to delete the
>> > function from the RDMA after print_on_off_bool() will be improved.
>>
>> The RDMAtool uses literal "on" and "off" as values in JSON, not
>> booleans. Moving over to print_on_off_bool() would be a breaking change,
>> which is problematic especially in JSON output.
>
> Nothing prohibits us from adding extra parameter to this new
> function/json logic/json type that will control JSON behavior. Personally,
> I don't think that json and stdout outputs should be different, e.g. 1/0 for
> the json and on/off for the stdout.

Emitting on/off in JSON as true booleans (true / false, not 1 / 0) does
make sense. It's programmatically-consumed interface, the values should
be of the right type.

On the other hand, having a FP output use literal "on" and "off" makes
sense as well. It's an obvious reference to the command line, you can
actually cut'n'paste it back to shell and it will do the right thing.

Many places in iproute2 do do this dual output, and ideally all new
instances would behave this way as well. So no toggles, please.

>> I think the current function does handle JSON context, what else do
>> you have in mind?
>
> It handles, but does it twice, first time for is_json_context() and
> second time inside print_bool.

Gotcha.