On Fri. 7 Oct. 2022 at 16:56, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> On 04.11.2021 01:44:26, Vincent Mailhol wrote:
> > Current implementation heavily relies on some "if (is_json_context())"
> > switches to decide the context and then does some print_*(PRINT_JSON,
> > ...) when in json context and some fprintf(...) else.
> >
> > Furthermore, current implementation uses either print_int() or the
> > conversion specifier %d to print unsigned integers.
> >
> > This patch factorizes each pairs of print_*(PRINT_JSON, ...) and
> > fprintf() into a single print_*(PRINT_ANY, ...) call. While doing this
> > replacement, it uses proper unsigned function print_uint() as well as
> > the conversion specifier %u when the parameter is an unsigned integer.
> >
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
>
> [...]
>
> > if (tb[IFLA_CAN_TERMINATION_CONST] && tb[IFLA_CAN_TERMINATION]) {
> > @@ -538,29 +483,21 @@ static void can_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
> > sizeof(*trm_const);
> > int i;
> >
> > - if (is_json_context()) {
> > - print_hu(PRINT_JSON, "termination", NULL, *trm);
> > - open_json_array(PRINT_JSON, "termination_const");
> > - for (i = 0; i < trm_cnt; ++i)
> > - print_hu(PRINT_JSON, NULL, NULL, trm_const[i]);
> > - close_json_array(PRINT_JSON, NULL);
> > - } else {
> > - fprintf(f, "\n termination %hu [ ", *trm);
> > -
> > - for (i = 0; i < trm_cnt - 1; ++i)
> > - fprintf(f, "%hu, ", trm_const[i]);
> > -
> > - fprintf(f, "%hu ]", trm_const[i]);
> ^
> > - }
> > + can_print_nl_indent();
> > + print_hu(PRINT_ANY, "termination", " termination %hu [ ", *trm);
>
> Always '['
>
> > + open_json_array(PRINT_JSON, "termination_const");
> > + for (i = 0; i < trm_cnt; ++i)
> > + print_hu(PRINT_ANY, NULL,
> > + i < trm_cnt - 1 ? "%hu, " : "%hu",
> > + trm_const[i]);
> > + close_json_array(PRINT_JSON, " ]");
>
> ']' only for JSON.
Thanks for the report
Actually, the second argument of close_json_array() is what should be
printed for the normal (non-json) output. Here it is correctly set to
" ]". *However*, this parameter gets ignored because the first
argument is PRINT_JSON instead of PRINT_ANY.
This is a lack of testing on my side (I do not have hardware which
supports the switchable termination resistors). After investigation,
the bitrate and dbitrate also have the same issue.
This is fixed in
https://lore.kernel.org/linux-can/20221010110118.66116-1-mailhol.vincent@xxxxxxxxxx/
> > }
>
> I just noticed that the non JSON output for termination is missing the
> closing ']'. See the output in the documentation update by Daniel:
>
> | https://lore.kernel.org/all/4514353.LvFx2qVVIh@daniel6430
Yours sincerely,
Vincent Mailhol
