| Message ID | 1418713218-16300-17-git-send-email-yszhou4tech@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2014-12-16 08:00, Yousong Zhou wrote: > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > --- > uci_internal.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/uci_internal.h b/uci_internal.h > index 89863f1..cb8f86c 100644 > --- a/uci_internal.h > +++ b/uci_internal.h > @@ -227,10 +227,10 @@ struct uci_backend _var = { \ > * Sets Exception handling to passthrough mode. > * Allows API functions to change behavior compared to public use > */ > -#define UCI_INTERNAL(func, ctx, ...) do { \ > - ctx->internal = true; \ > - func(ctx, __VA_ARGS__); \ > -} while (0) > +#define UCI_INTERNAL(func, ctx, ...) ( \ > + ctx->internal = true, \ > + func(ctx, __VA_ARGS__) \ > +) A patch like this should have a description that explains why the change was made. If I had to guess, I'd say you intend to make it possible to use the return code of UCI_INTERNAL(...). The standard way to do that is to write it as ({ ctx->internal = true; func(...); }) - Felix
On 18 December 2014 at 19:08, Felix Fietkau <nbd@openwrt.org> wrote: > On 2014-12-16 08:00, Yousong Zhou wrote: >> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> >> --- >> uci_internal.h | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/uci_internal.h b/uci_internal.h >> index 89863f1..cb8f86c 100644 >> --- a/uci_internal.h >> +++ b/uci_internal.h >> @@ -227,10 +227,10 @@ struct uci_backend _var = { \ >> * Sets Exception handling to passthrough mode. >> * Allows API functions to change behavior compared to public use >> */ >> -#define UCI_INTERNAL(func, ctx, ...) do { \ >> - ctx->internal = true; \ >> - func(ctx, __VA_ARGS__); \ >> -} while (0) >> +#define UCI_INTERNAL(func, ctx, ...) ( \ >> + ctx->internal = true, \ >> + func(ctx, __VA_ARGS__) \ >> +) > A patch like this should have a description that explains why the change > was made. If I had to guess, I'd say you intend to make it possible to > use the return code of UCI_INTERNAL(...). Indeed. Thought I need to check the return value of uci_parse_argument(), then wondering this change should not hurt. > The standard way to do that is to write it as > ({ ctx->internal = true; func(...); }) Thank you. Was not aware of this GCC extension before. Regards. yousong
diff --git a/uci_internal.h b/uci_internal.h index 89863f1..cb8f86c 100644 --- a/uci_internal.h +++ b/uci_internal.h @@ -227,10 +227,10 @@ struct uci_backend _var = { \ * Sets Exception handling to passthrough mode. * Allows API functions to change behavior compared to public use */ -#define UCI_INTERNAL(func, ctx, ...) do { \ - ctx->internal = true; \ - func(ctx, __VA_ARGS__); \ -} while (0) +#define UCI_INTERNAL(func, ctx, ...) ( \ + ctx->internal = true, \ + func(ctx, __VA_ARGS__) \ +) /** * UCI_NESTED: Do an normal nested call of a public API function
Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> --- uci_internal.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)