Message ID | 1549823329-10377-3-git-send-email-ayal@mellanox.com |
---|---|
State | Changes Requested |
Delegated to: | David Ahern |
Headers | show |
Series | Add support for devlink health | expand |
On Sun, 10 Feb 2019 20:28:47 +0200 Aya Levin <ayal@mellanox.com> wrote: > This patch prints uint64_t with its corresponding format and avoid implicit > cast to uint32_t. > > Signed-off-by: Aya Levin <ayal@mellanox.com> > Reviewed-by: Moshe Shemesh <moshe@mellanox.com> > Reported-by: Maria Pasechnik <mariap@mellanox.com> > --- > devlink/devlink.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/devlink/devlink.c b/devlink/devlink.c > index a05755385a49..46e2e41c5dfd 100644 > --- a/devlink/devlink.c > +++ b/devlink/devlink.c > @@ -1628,7 +1628,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val) > if (val == (uint64_t) -1) > return pr_out_str(dl, name, "unlimited"); > > - return pr_out_uint(dl, name, val); > + if (dl->json_output) { > + jsonw_u64_field(dl->jw, name, val); > + } else { > + if (g_indent_newline) > + pr_out("%s %lu", name, val); > + else > + pr_out(" %s %lu", name, val); > + } > } > > static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) More conditional code adds bloat and is a nuisance. Why not use print_u64 that already exists. I wish devlink used json_print in a more standard manner.
On 2/10/19 1:34 PM, Stephen Hemminger wrote: > On Sun, 10 Feb 2019 20:28:47 +0200 > Aya Levin <ayal@mellanox.com> wrote: > >> This patch prints uint64_t with its corresponding format and avoid implicit >> cast to uint32_t. >> >> Signed-off-by: Aya Levin <ayal@mellanox.com> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com> >> Reported-by: Maria Pasechnik <mariap@mellanox.com> >> --- >> devlink/devlink.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/devlink/devlink.c b/devlink/devlink.c >> index a05755385a49..46e2e41c5dfd 100644 >> --- a/devlink/devlink.c >> +++ b/devlink/devlink.c >> @@ -1628,7 +1628,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val) >> if (val == (uint64_t) -1) >> return pr_out_str(dl, name, "unlimited"); >> >> - return pr_out_uint(dl, name, val); >> + if (dl->json_output) { >> + jsonw_u64_field(dl->jw, name, val); >> + } else { >> + if (g_indent_newline) >> + pr_out("%s %lu", name, val); >> + else >> + pr_out(" %s %lu", name, val); I would expect the compiler to throw a warning when %lu is used for u64's. >> + } >> } >> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) > > More conditional code adds bloat and is a nuisance. > Why not use print_u64 that already exists. > > I wish devlink used json_print in a more standard manner. > That's a general devlink problem that can be addressed outside of this patch set. It is an example of what I meant by devlink not using more of the infra from the iproute2 code.
Sun, Feb 10, 2019 at 07:28:47PM CET, ayal@mellanox.com wrote: > This patch prints uint64_t with its corresponding format and avoid implicit > cast to uint32_t. Drop the space at the beginning of the lines. Otherwise this looks fine. Acked-by: Jiri Pirko <jiri@mellanox.com>
diff --git a/devlink/devlink.c b/devlink/devlink.c index a05755385a49..46e2e41c5dfd 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -1628,7 +1628,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val) if (val == (uint64_t) -1) return pr_out_str(dl, name, "unlimited"); - return pr_out_uint(dl, name, val); + if (dl->json_output) { + jsonw_u64_field(dl->jw, name, val); + } else { + if (g_indent_newline) + pr_out("%s %lu", name, val); + else + pr_out(" %s %lu", name, val); + } } static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)