| Message ID | 20231010104318.3571791-2-przemyslaw.kitszel@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | devlink: retain error in struct devlink_fmsg | expand |
Tue, Oct 10, 2023 at 12:43:11PM CEST, przemyslaw.kitszel@intel.com wrote: >Retain error value in struct devlink_fmsg, to relieve drivers from >checking it after each call. >Note that fmsg is an in-memory builder/buffer of formatted message, >so it's not the case that half baked message was sent somewhere. > >We could find following scheme in multiple drivers: > err = devlink_fmsg_obj_nest_start(fmsg); > if (err) > return err; > err = devlink_fmsg_string_pair_put(fmsg, "src", src); > if (err) > return err; > err = devlink_fmsg_something(fmsg, foo, bar); > if (err) > return err; > // and so on... > err = devlink_fmsg_obj_nest_end(fmsg); > >With retaining error API that translates to: > devlink_fmsg_obj_nest_start(fmsg); > devlink_fmsg_string_pair_put(fmsg, "src", src); > devlink_fmsg_something(fmsg, foo, bar); > // and so on... > err = devlink_fmsg_obj_nest_end(fmsg); I like this approach. But it looks a bit odd that you store error and return it as well, leaving the caller to decide what to do in his code. It is not desirable to leave the caller wondering. Also, it is customary to check the return value if the function returns it. This approach confuses the customs. Also, eventually, the fmsg is getting send. That is the point where the error could be checked and handled properly, for example by filling nice extack message. What I'm saying is, please convert them all to return void, store the error and check that before fmsg send. That makes the approach unified for all callers, code nicer. Even the custom in-driver put functions would return void. The callbacks (e. g. dump) would also return void. + a small nit below: > >What means we check error just at the end >(one could return it directly of course). > >Possible error scenarios are developer error (API misuse) and memory >exhaustion, both cases are good candidates to choose readability >over fastest possible exit. > >This commit itself is an illustration of benefits for the dev-user, >more of it will be in separate commits of the series. > >Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> >Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> >--- >add/remove: 2/4 grow/shrink: 11/9 up/down: 325/-646 (-321) >--- > net/devlink/health.c | 255 ++++++++++++++++--------------------------- > 1 file changed, 92 insertions(+), 163 deletions(-) > >diff --git a/net/devlink/health.c b/net/devlink/health.c >index 638cad8d5c65..2d26479e9dbe 100644 >--- a/net/devlink/health.c >+++ b/net/devlink/health.c >@@ -19,6 +19,7 @@ struct devlink_fmsg_item { > > struct devlink_fmsg { > struct list_head item_list; >+ int err; /* first error encountered on some devlink_fmsg_XXX() call */ > bool putting_binary; /* This flag forces enclosing of binary data > * in an array brackets. It forces using > * of designated API: >@@ -565,10 +566,8 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter, > return 0; > > reporter->dump_fmsg = devlink_fmsg_alloc(); >- if (!reporter->dump_fmsg) { >- err = -ENOMEM; >- return err; >- } >+ if (!reporter->dump_fmsg) >+ return -ENOMEM; > > err = devlink_fmsg_obj_nest_start(reporter->dump_fmsg); > if (err) >@@ -673,43 +672,59 @@ int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, > return devlink_health_reporter_recover(reporter, NULL, info->extack); > } > >-static int devlink_fmsg_nest_common(struct devlink_fmsg *fmsg, >- int attrtype) >+static bool _devlink_fmsg_err_or_binary(struct devlink_fmsg *fmsg) No need for "_" here. Drop it. >+{ >+ if (!fmsg->err && fmsg->putting_binary) >+ fmsg->err = -EINVAL; >+ >+ return fmsg->err; >+} >+ [...]
On 10/10/23 13:34, Jiri Pirko wrote: > Tue, Oct 10, 2023 at 12:43:11PM CEST, przemyslaw.kitszel@intel.com wrote: >> Retain error value in struct devlink_fmsg, to relieve drivers from >> checking it after each call. >> Note that fmsg is an in-memory builder/buffer of formatted message, >> so it's not the case that half baked message was sent somewhere. >> >> We could find following scheme in multiple drivers: >> err = devlink_fmsg_obj_nest_start(fmsg); >> if (err) >> return err; >> err = devlink_fmsg_string_pair_put(fmsg, "src", src); >> if (err) >> return err; >> err = devlink_fmsg_something(fmsg, foo, bar); >> if (err) >> return err; >> // and so on... >> err = devlink_fmsg_obj_nest_end(fmsg); >> >> With retaining error API that translates to: >> devlink_fmsg_obj_nest_start(fmsg); >> devlink_fmsg_string_pair_put(fmsg, "src", src); >> devlink_fmsg_something(fmsg, foo, bar); >> // and so on... >> err = devlink_fmsg_obj_nest_end(fmsg); > > I like this approach. But it looks a bit odd that you store error and > return it as well, leaving the caller to decide what to do in his code. > It is not desirable to leave the caller wondering. > > Also, it is customary to check the return value if the function returns > it. This approach confuses the customs. > > Also, eventually, the fmsg is getting send. That is the point where the > error could be checked and handled properly, for example by filling nice > extack message. > > What I'm saying is, please convert them all to return void, store the > error and check that before fmsg send. That makes the approach unified > for all callers, code nicer. Even the custom in-driver put functions > would return void. The callbacks (e. g. dump) would also return void. I was also thinking about that, what about cases that you want to exit early, say inside of some loop? add also devlink_fmsg_is_err()? anyway, I like results more with ultimate unification :), only then all the drivers require conversion at the very same time > > + a small nit below: > > >> >> What means we check error just at the end >> (one could return it directly of course). >> >> Possible error scenarios are developer error (API misuse) and memory >> exhaustion, both cases are good candidates to choose readability >> over fastest possible exit. >> >> This commit itself is an illustration of benefits for the dev-user, >> more of it will be in separate commits of the series. >> >> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> >> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> >> --- >> add/remove: 2/4 grow/shrink: 11/9 up/down: 325/-646 (-321) >> --- >> net/devlink/health.c | 255 ++++++++++++++++--------------------------- >> 1 file changed, 92 insertions(+), 163 deletions(-) >> >> diff --git a/net/devlink/health.c b/net/devlink/health.c >> index 638cad8d5c65..2d26479e9dbe 100644 >> --- a/net/devlink/health.c >> +++ b/net/devlink/health.c >> @@ -19,6 +19,7 @@ struct devlink_fmsg_item { >> >> struct devlink_fmsg { >> struct list_head item_list; >> + int err; /* first error encountered on some devlink_fmsg_XXX() call */ >> bool putting_binary; /* This flag forces enclosing of binary data >> * in an array brackets. It forces using >> * of designated API: >> @@ -565,10 +566,8 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter, >> return 0; >> >> reporter->dump_fmsg = devlink_fmsg_alloc(); >> - if (!reporter->dump_fmsg) { >> - err = -ENOMEM; >> - return err; >> - } >> + if (!reporter->dump_fmsg) >> + return -ENOMEM; >> >> err = devlink_fmsg_obj_nest_start(reporter->dump_fmsg); >> if (err) >> @@ -673,43 +672,59 @@ int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, >> return devlink_health_reporter_recover(reporter, NULL, info->extack); >> } >> >> -static int devlink_fmsg_nest_common(struct devlink_fmsg *fmsg, >> - int attrtype) >> +static bool _devlink_fmsg_err_or_binary(struct devlink_fmsg *fmsg) > > No need for "_" here. Drop it. > > >> +{ >> + if (!fmsg->err && fmsg->putting_binary) >> + fmsg->err = -EINVAL; >> + >> + return fmsg->err; >> +} >> + > > [...]
Tue, Oct 10, 2023 at 03:36:48PM CEST, przemyslaw.kitszel@intel.com wrote: >On 10/10/23 13:34, Jiri Pirko wrote: >> Tue, Oct 10, 2023 at 12:43:11PM CEST, przemyslaw.kitszel@intel.com wrote: >> > Retain error value in struct devlink_fmsg, to relieve drivers from >> > checking it after each call. >> > Note that fmsg is an in-memory builder/buffer of formatted message, >> > so it's not the case that half baked message was sent somewhere. >> > >> > We could find following scheme in multiple drivers: >> > err = devlink_fmsg_obj_nest_start(fmsg); >> > if (err) >> > return err; >> > err = devlink_fmsg_string_pair_put(fmsg, "src", src); >> > if (err) >> > return err; >> > err = devlink_fmsg_something(fmsg, foo, bar); >> > if (err) >> > return err; >> > // and so on... >> > err = devlink_fmsg_obj_nest_end(fmsg); >> > >> > With retaining error API that translates to: >> > devlink_fmsg_obj_nest_start(fmsg); >> > devlink_fmsg_string_pair_put(fmsg, "src", src); >> > devlink_fmsg_something(fmsg, foo, bar); >> > // and so on... >> > err = devlink_fmsg_obj_nest_end(fmsg); >> >> I like this approach. But it looks a bit odd that you store error and >> return it as well, leaving the caller to decide what to do in his code. >> It is not desirable to leave the caller wondering. >> >> Also, it is customary to check the return value if the function returns >> it. This approach confuses the customs. >> >> Also, eventually, the fmsg is getting send. That is the point where the >> error could be checked and handled properly, for example by filling nice >> extack message. >> >> What I'm saying is, please convert them all to return void, store the >> error and check that before fmsg send. That makes the approach unified >> for all callers, code nicer. Even the custom in-driver put functions >> would return void. The callbacks (e. g. dump) would also return void. > >I was also thinking about that, >what about cases that you want to exit early, say inside of some loop? Why would you need that? As you stated yourself, the error might happen when driver is buggy or under memory pressure. So in these cases, we don't exit early. So? I believe that we don't have to care about this corner cases. >add also devlink_fmsg_is_err()? > >anyway, I like results more with ultimate unification :), only then all the >drivers require conversion at the very same time > >> >> + a small nit below: >> >> >> > >> > What means we check error just at the end >> > (one could return it directly of course). >> > >> > Possible error scenarios are developer error (API misuse) and memory >> > exhaustion, both cases are good candidates to choose readability >> > over fastest possible exit. >> > >> > This commit itself is an illustration of benefits for the dev-user, >> > more of it will be in separate commits of the series. >> > >> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> >> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> >> > --- >> > add/remove: 2/4 grow/shrink: 11/9 up/down: 325/-646 (-321) >> > --- >> > net/devlink/health.c | 255 ++++++++++++++++--------------------------- >> > 1 file changed, 92 insertions(+), 163 deletions(-) >> > >> > diff --git a/net/devlink/health.c b/net/devlink/health.c >> > index 638cad8d5c65..2d26479e9dbe 100644 >> > --- a/net/devlink/health.c >> > +++ b/net/devlink/health.c >> > @@ -19,6 +19,7 @@ struct devlink_fmsg_item { >> > >> > struct devlink_fmsg { >> > struct list_head item_list; >> > + int err; /* first error encountered on some devlink_fmsg_XXX() call */ >> > bool putting_binary; /* This flag forces enclosing of binary data >> > * in an array brackets. It forces using >> > * of designated API: >> > @@ -565,10 +566,8 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter, >> > return 0; >> > >> > reporter->dump_fmsg = devlink_fmsg_alloc(); >> > - if (!reporter->dump_fmsg) { >> > - err = -ENOMEM; >> > - return err; >> > - } >> > + if (!reporter->dump_fmsg) >> > + return -ENOMEM; >> > >> > err = devlink_fmsg_obj_nest_start(reporter->dump_fmsg); >> > if (err) >> > @@ -673,43 +672,59 @@ int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, >> > return devlink_health_reporter_recover(reporter, NULL, info->extack); >> > } >> > >> > -static int devlink_fmsg_nest_common(struct devlink_fmsg *fmsg, >> > - int attrtype) >> > +static bool _devlink_fmsg_err_or_binary(struct devlink_fmsg *fmsg) >> >> No need for "_" here. Drop it. >> >> >> > +{ >> > + if (!fmsg->err && fmsg->putting_binary) >> > + fmsg->err = -EINVAL; >> > + >> > + return fmsg->err; >> > +} >> > + >> >> [...] >
diff --git a/net/devlink/health.c b/net/devlink/health.c index 638cad8d5c65..2d26479e9dbe 100644 --- a/net/devlink/health.c +++ b/net/devlink/health.c @@ -19,6 +19,7 @@ struct devlink_fmsg_item { struct devlink_fmsg { struct list_head item_list; + int err; /* first error encountered on some devlink_fmsg_XXX() call */ bool putting_binary; /* This flag forces enclosing of binary data * in an array brackets. It forces using * of designated API: @@ -565,10 +566,8 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter, return 0; reporter->dump_fmsg = devlink_fmsg_alloc(); - if (!reporter->dump_fmsg) { - err = -ENOMEM; - return err; - } + if (!reporter->dump_fmsg) + return -ENOMEM; err = devlink_fmsg_obj_nest_start(reporter->dump_fmsg); if (err) @@ -673,43 +672,59 @@ int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, return devlink_health_reporter_recover(reporter, NULL, info->extack); } -static int devlink_fmsg_nest_common(struct devlink_fmsg *fmsg, - int attrtype) +static bool _devlink_fmsg_err_or_binary(struct devlink_fmsg *fmsg) +{ + if (!fmsg->err && fmsg->putting_binary) + fmsg->err = -EINVAL; + + return fmsg->err; +} + +static int devlink_fmsg_nest_common(struct devlink_fmsg *fmsg, int attrtype) { struct devlink_fmsg_item *item; + if (fmsg->err) + return fmsg->err; + item = kzalloc(sizeof(*item), GFP_KERNEL); - if (!item) - return -ENOMEM; + if (!item) { + fmsg->err = -ENOMEM; + return fmsg->err; + } item->attrtype = attrtype; list_add_tail(&item->list, &fmsg->item_list); return 0; } +/** + * devlink_fmsg_obj_nest_start - emit start of nested object to @fmsg + * + * @fmsg: struct devlink_fmsg pointer, formatted message + * + * If called on @fmsg already in error state, that error will be returned. + */ int devlink_fmsg_obj_nest_start(struct devlink_fmsg *fmsg) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_OBJ_NEST_START); } EXPORT_SYMBOL_GPL(devlink_fmsg_obj_nest_start); static int devlink_fmsg_nest_end(struct devlink_fmsg *fmsg) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_NEST_END); } int devlink_fmsg_obj_nest_end(struct devlink_fmsg *fmsg) { - if (fmsg->putting_binary) - return -EINVAL; - return devlink_fmsg_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_obj_nest_end); @@ -720,15 +735,19 @@ static int devlink_fmsg_put_name(struct devlink_fmsg *fmsg, const char *name) { struct devlink_fmsg_item *item; - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; - if (strlen(name) + 1 > DEVLINK_FMSG_MAX_SIZE) - return -EMSGSIZE; + if (strlen(name) + 1 > DEVLINK_FMSG_MAX_SIZE) { + fmsg->err = -EMSGSIZE; + return fmsg->err; + } item = kzalloc(sizeof(*item) + strlen(name) + 1, GFP_KERNEL); - if (!item) - return -ENOMEM; + if (!item) { + fmsg->err = -ENOMEM; + return fmsg->err; + } item->nla_type = NLA_NUL_STRING; item->len = strlen(name) + 1; @@ -741,68 +760,32 @@ static int devlink_fmsg_put_name(struct devlink_fmsg *fmsg, const char *name) int devlink_fmsg_pair_nest_start(struct devlink_fmsg *fmsg, const char *name) { - int err; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; - if (fmsg->putting_binary) - return -EINVAL; - - err = devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_PAIR_NEST_START); - if (err) - return err; - - err = devlink_fmsg_put_name(fmsg, name); - if (err) - return err; - - return 0; + devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_PAIR_NEST_START); + return devlink_fmsg_put_name(fmsg, name); } EXPORT_SYMBOL_GPL(devlink_fmsg_pair_nest_start); int devlink_fmsg_pair_nest_end(struct devlink_fmsg *fmsg) { - if (fmsg->putting_binary) - return -EINVAL; - return devlink_fmsg_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_pair_nest_end); int devlink_fmsg_arr_pair_nest_start(struct devlink_fmsg *fmsg, const char *name) { - int err; - - if (fmsg->putting_binary) - return -EINVAL; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_ARR_NEST_START); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + return devlink_fmsg_nest_common(fmsg, DEVLINK_ATTR_FMSG_ARR_NEST_START); } EXPORT_SYMBOL_GPL(devlink_fmsg_arr_pair_nest_start); int devlink_fmsg_arr_pair_nest_end(struct devlink_fmsg *fmsg) { - int err; - - if (fmsg->putting_binary) - return -EINVAL; - - err = devlink_fmsg_nest_end(fmsg); - if (err) - return err; - - err = devlink_fmsg_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_nest_end(fmsg); + return devlink_fmsg_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_arr_pair_nest_end); @@ -816,14 +799,19 @@ int devlink_fmsg_binary_pair_nest_start(struct devlink_fmsg *fmsg, return err; fmsg->putting_binary = true; - return err; + return 0; } EXPORT_SYMBOL_GPL(devlink_fmsg_binary_pair_nest_start); int devlink_fmsg_binary_pair_nest_end(struct devlink_fmsg *fmsg) { - if (!fmsg->putting_binary) - return -EINVAL; + if (fmsg->err) + return fmsg->err; + + if (!fmsg->err && !fmsg->putting_binary) { + fmsg->err = -EINVAL; + return fmsg->err; + } fmsg->putting_binary = false; return devlink_fmsg_arr_pair_nest_end(fmsg); @@ -836,12 +824,16 @@ static int devlink_fmsg_put_value(struct devlink_fmsg *fmsg, { struct devlink_fmsg_item *item; - if (value_len > DEVLINK_FMSG_MAX_SIZE) - return -EMSGSIZE; + if (value_len > DEVLINK_FMSG_MAX_SIZE) { + fmsg->err = -EMSGSIZE; + return fmsg->err; + } item = kzalloc(sizeof(*item) + value_len, GFP_KERNEL); - if (!item) - return -ENOMEM; + if (!item) { + fmsg->err = -ENOMEM; + return fmsg->err; + } item->nla_type = value_nla_type; item->len = value_len; @@ -854,41 +846,41 @@ static int devlink_fmsg_put_value(struct devlink_fmsg *fmsg, static int devlink_fmsg_bool_put(struct devlink_fmsg *fmsg, bool value) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_FLAG); } static int devlink_fmsg_u8_put(struct devlink_fmsg *fmsg, u8 value) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U8); } int devlink_fmsg_u32_put(struct devlink_fmsg *fmsg, u32 value) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U32); } EXPORT_SYMBOL_GPL(devlink_fmsg_u32_put); static int devlink_fmsg_u64_put(struct devlink_fmsg *fmsg, u64 value) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U64); } int devlink_fmsg_string_put(struct devlink_fmsg *fmsg, const char *value) { - if (fmsg->putting_binary) - return -EINVAL; + if (_devlink_fmsg_err_or_binary(fmsg)) + return fmsg->err; return devlink_fmsg_put_value(fmsg, value, strlen(value) + 1, NLA_NUL_STRING); @@ -908,113 +900,52 @@ EXPORT_SYMBOL_GPL(devlink_fmsg_binary_put); int devlink_fmsg_bool_pair_put(struct devlink_fmsg *fmsg, const char *name, bool value) { - int err; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_bool_put(fmsg, value); - if (err) - return err; - - err = devlink_fmsg_pair_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + devlink_fmsg_bool_put(fmsg, value); + return devlink_fmsg_pair_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_bool_pair_put); int devlink_fmsg_u8_pair_put(struct devlink_fmsg *fmsg, const char *name, u8 value) { - int err; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_u8_put(fmsg, value); - if (err) - return err; - - err = devlink_fmsg_pair_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + devlink_fmsg_u8_put(fmsg, value); + return devlink_fmsg_pair_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_u8_pair_put); int devlink_fmsg_u32_pair_put(struct devlink_fmsg *fmsg, const char *name, u32 value) { - int err; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_u32_put(fmsg, value); - if (err) - return err; - - err = devlink_fmsg_pair_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + devlink_fmsg_u32_put(fmsg, value); + return devlink_fmsg_pair_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_u32_pair_put); int devlink_fmsg_u64_pair_put(struct devlink_fmsg *fmsg, const char *name, u64 value) { - int err; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_u64_put(fmsg, value); - if (err) - return err; - - err = devlink_fmsg_pair_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + devlink_fmsg_u64_put(fmsg, value); + return devlink_fmsg_pair_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_u64_pair_put); int devlink_fmsg_string_pair_put(struct devlink_fmsg *fmsg, const char *name, const char *value) { - int err; - - err = devlink_fmsg_pair_nest_start(fmsg, name); - if (err) - return err; - - err = devlink_fmsg_string_put(fmsg, value); - if (err) - return err; - - err = devlink_fmsg_pair_nest_end(fmsg); - if (err) - return err; - - return 0; + devlink_fmsg_pair_nest_start(fmsg, name); + devlink_fmsg_string_put(fmsg, value); + return devlink_fmsg_pair_nest_end(fmsg); } EXPORT_SYMBOL_GPL(devlink_fmsg_string_pair_put); int devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name, const void *value, u32 value_len) { u32 data_size; - int end_err; u32 offset; int err; @@ -1030,14 +961,12 @@ int devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name, if (err) break; /* Exit from loop with a break (instead of - * return) to make sure putting_binary is turned off in - * devlink_fmsg_binary_pair_nest_end + * return) to make sure putting_binary is turned off */ } - end_err = devlink_fmsg_binary_pair_nest_end(fmsg); - if (end_err) - err = end_err; + err = devlink_fmsg_binary_pair_nest_end(fmsg); + fmsg->putting_binary = false; return err; }