On Fri. 7 Mai 2021 at 00:50, Stephen Hemminger
[off-list ref] wrote:

On Thu,  6 May 2021 20:20:07 +0900
Vincent Mailhol [off-list ref] wrote:

quoted

+     if (tb[IFLA_CAN_TDC_TDCV] && tb[IFLA_CAN_TDC_TDCO] &&
+         tb[IFLA_CAN_TDC_TDCF]) {
+             __u32 *tdcv = RTA_DATA(tb[IFLA_CAN_TDC_TDCV]);
+             __u32 *tdco = RTA_DATA(tb[IFLA_CAN_TDC_TDCO]);
+             __u32 *tdcf = RTA_DATA(tb[IFLA_CAN_TDC_TDCF]);
+
+             if (is_json_context()) {
+                     open_json_object("tdc");
+                     print_int(PRINT_JSON, "tdcv", NULL, *tdcv);
+                     print_int(PRINT_JSON, "tdco", NULL, *tdco);
+                     print_int(PRINT_JSON, "tdcf", NULL, *tdcf);
+                     close_json_object();
+             } else {
+                     fprintf(f, "\n    tdcv %d tdco %d tdcf %d",
+                             *tdcv, *tdco, *tdcf);
+             }
+     }
+

The most common pattern in iproute2 is to let json/non-json be decided
inside the print routine.  I search for all instances of fprintf as
indication of broken code. Also these are not signed values so please
print unsigned.  The code should use print_nl() to handle the single line
case. Also, there is helper to handle

Thanks for pointing out the issues!  For my defence, all the
existing code of ip/iplink_can.c shares the exact same issues and
I just repeated those without using my brain...

I just sent a new v2 which fixes your remarks on both the
existing code and the new code introduced in my patch.

Something like:
              __u32 tdc = rte_getattr_u32(tb[IFLA_CAN_TDC_TDCV]);

                open_json_object("tdc");
                print_nl();
                print_u32(PRINT_ANY, "tdcv", "    tdcv %u", tdcv);

There is no print_u32 in iproute2. I guess you meant print_uint(). :)


Yours sincerely,
Vincent