On Tue, Nov 03, 2020 at 10:01:32PM +0100, Petr Machata wrote: > > Leon Romanovsky <l...@kernel.org> writes: > > > On Tue, Nov 03, 2020 at 12:05:20AM +0100, Petr Machata wrote: > >> > >> Leon Romanovsky <l...@kernel.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.
As long as you don't need to use those fields to "set .." after that. > > 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. Maybe it is not so bad to change RDMAtool to general function, this on/of print is not widely use yet, just need to decide what is the right one. > > Many places in iproute2 do do this dual output, and ideally all new > instances would behave this way as well. So no toggles, please. Good example why all utilities in iproute2 are better to use same input/output code and any attempt to make custom variants should be banned. > > >> 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.
