* [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c @ 2015-06-27 4:44 Vasiliy Korchagin 2015-06-27 4:53 ` Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: Vasiliy Korchagin @ 2015-06-27 4:44 UTC (permalink / raw) To: oleg.drokin, andreas.dilger Cc: HPDD-discuss, devel, linux-kernel, Vasiliy Korchagin This patch fixes the checkpatch.pl error: ERROR: Macros with complex values should be enclosed in parentheses +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) Signed-off-by: Vasiliy Korchagin <vasiliy.korchagin@gmail.com> --- drivers/staging/lustre/lustre/obdclass/uuid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..8d00882 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -55,7 +55,7 @@ static inline __u32 consume(int nob, __u8 **ptr) return value; } -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) +#define CONSUME(val, ptr) ((val) = consume(sizeof(val), (ptr))) static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) { -- 2.4.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c 2015-06-27 4:44 [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c Vasiliy Korchagin @ 2015-06-27 4:53 ` Joe Perches 2015-06-27 5:36 ` Vasiliy Korchagin 0 siblings, 1 reply; 7+ messages in thread From: Joe Perches @ 2015-06-27 4:53 UTC (permalink / raw) To: Vasiliy Korchagin Cc: oleg.drokin, andreas.dilger, HPDD-discuss, devel, linux-kernel On Sat, 2015-06-27 at 05:44 +0100, Vasiliy Korchagin wrote: > This patch fixes the checkpatch.pl error: > > ERROR: Macros with complex values should be enclosed in parentheses > +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > > Signed-off-by: Vasiliy Korchagin <vasiliy.korchagin@gmail.com> > --- > drivers/staging/lustre/lustre/obdclass/uuid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c > index ff0a01b..8d00882 100644 > --- a/drivers/staging/lustre/lustre/obdclass/uuid.c > +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c > @@ -55,7 +55,7 @@ static inline __u32 consume(int nob, __u8 **ptr) > return value; > } > > -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > +#define CONSUME(val, ptr) ((val) = consume(sizeof(val), (ptr))) CONSUME is used exactly once. It'd be likely be simpler to remove it and expand it in-place instead. The static inline __u32 consume() function is also used once and might as well be expanded in-place too. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c 2015-06-27 4:53 ` Joe Perches @ 2015-06-27 5:36 ` Vasiliy Korchagin 2015-06-27 6:04 ` Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: Vasiliy Korchagin @ 2015-06-27 5:36 UTC (permalink / raw) To: oleg.drokin, andreas.dilger, joe; +Cc: devel, linux-kernel, Vasiliy Korchagin This patch fixes the checkpatch.pl error: ERROR: Macros with complex values should be enclosed in parentheses +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) by expanding it as this macro is used only once. Signed-off-by: Vasiliy Korchagin <vasiliy.korchagin@gmail.com> --- Notes: Here is another version with macro expansion. Inline function expansion doesn't seem like a good idea to me as it would make things overcomplicated. drivers/staging/lustre/lustre/obdclass/uuid.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..d121c5c 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -55,8 +55,6 @@ static inline __u32 consume(int nob, __u8 **ptr) return value; } -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) - static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) { __u8 *ptr = in; @@ -64,7 +62,7 @@ static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); while (nr-- > 0) - CONSUME(uu[nr], &ptr); + uu[nr] = consume(sizeof(uu[nr]), &ptr); } void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) -- 2.4.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c 2015-06-27 5:36 ` Vasiliy Korchagin @ 2015-06-27 6:04 ` Joe Perches 2015-07-07 2:36 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Joe Perches @ 2015-06-27 6:04 UTC (permalink / raw) To: Vasiliy Korchagin; +Cc: oleg.drokin, andreas.dilger, devel, linux-kernel On Sat, 2015-06-27 at 06:36 +0100, Vasiliy Korchagin wrote: > This patch fixes the checkpatch.pl error: > > ERROR: Macros with complex values should be enclosed in parentheses > +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > > by expanding it as this macro is used only once. [] > Notes: > Here is another version with macro expansion. Inline function expansion doesn't > seem like a good idea to me as it would make things overcomplicated. It looks like it'd be simpler to use vsprintf extension %pU --- drivers/staging/lustre/lustre/obdclass/uuid.c | 34 +-------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..b0b0157 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -43,40 +43,8 @@ #include "../include/obd_support.h" #include "../include/obd_class.h" - -static inline __u32 consume(int nob, __u8 **ptr) -{ - __u32 value; - - LASSERT(nob <= sizeof(value)); - - for (value = 0; nob > 0; --nob) - value = (value << 8) | *((*ptr)++); - return value; -} - -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) - -static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) -{ - __u8 *ptr = in; - - LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); - - while (nr-- > 0) - CONSUME(uu[nr], &ptr); -} - void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) { - /* uu as an array of __u16's */ - __u16 uuid[sizeof(class_uuid_t) / sizeof(__u16)]; - - CLASSERT(ARRAY_SIZE(uuid) == 8); - - uuid_unpack(uu, uuid, ARRAY_SIZE(uuid)); - sprintf(out->uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7]); + sprintf(out->uuid, "%pU", uu); } EXPORT_SYMBOL(class_uuid_unparse); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c 2015-06-27 6:04 ` Joe Perches @ 2015-07-07 2:36 ` Greg KH 2015-07-07 5:01 ` Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2015-07-07 2:36 UTC (permalink / raw) To: Joe Perches Cc: Vasiliy Korchagin, oleg.drokin, devel, andreas.dilger, linux-kernel On Fri, Jun 26, 2015 at 11:04:49PM -0700, Joe Perches wrote: > On Sat, 2015-06-27 at 06:36 +0100, Vasiliy Korchagin wrote: > > This patch fixes the checkpatch.pl error: > > > > ERROR: Macros with complex values should be enclosed in parentheses > > +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > > > > by expanding it as this macro is used only once. > [] > > Notes: > > Here is another version with macro expansion. Inline function expansion doesn't > > seem like a good idea to me as it would make things overcomplicated. > > It looks like it'd be simpler to use vsprintf extension %pU > --- > drivers/staging/lustre/lustre/obdclass/uuid.c | 34 +-------------------------- > 1 file changed, 1 insertion(+), 33 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c > index ff0a01b..b0b0157 100644 > --- a/drivers/staging/lustre/lustre/obdclass/uuid.c > +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c > @@ -43,40 +43,8 @@ > #include "../include/obd_support.h" > #include "../include/obd_class.h" > > - > -static inline __u32 consume(int nob, __u8 **ptr) > -{ > - __u32 value; > - > - LASSERT(nob <= sizeof(value)); > - > - for (value = 0; nob > 0; --nob) > - value = (value << 8) | *((*ptr)++); > - return value; > -} > - > -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > - > -static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) > -{ > - __u8 *ptr = in; > - > - LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); > - > - while (nr-- > 0) > - CONSUME(uu[nr], &ptr); > -} > - > void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) > { > - /* uu as an array of __u16's */ > - __u16 uuid[sizeof(class_uuid_t) / sizeof(__u16)]; > - > - CLASSERT(ARRAY_SIZE(uuid) == 8); > - > - uuid_unpack(uu, uuid, ARRAY_SIZE(uuid)); > - sprintf(out->uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", > - uuid[0], uuid[1], uuid[2], uuid[3], > - uuid[4], uuid[5], uuid[6], uuid[7]); > + sprintf(out->uuid, "%pU", uu); > } > EXPORT_SYMBOL(class_uuid_unparse); I agree, much better, can you resend this in a form I can apply? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c 2015-07-07 2:36 ` Greg KH @ 2015-07-07 5:01 ` Joe Perches 2015-07-15 0:40 ` [PATCH] staging: lustre: obdclass: simplify class_uuid_unparse Vasiliy Korchagin 0 siblings, 1 reply; 7+ messages in thread From: Joe Perches @ 2015-07-07 5:01 UTC (permalink / raw) To: Greg KH; +Cc: Vasiliy Korchagin, oleg.drokin, devel, andreas.dilger, linux-kernel On Mon, 2015-07-06 at 19:36 -0700, Greg KH wrote: > On Fri, Jun 26, 2015 at 11:04:49PM -0700, Joe Perches wrote: > > On Sat, 2015-06-27 at 06:36 +0100, Vasiliy Korchagin wrote: [] > > > Here is another version with macro expansion. Inline function expansion doesn't > > > seem like a good idea to me as it would make things overcomplicated. > > > > It looks like it'd be simpler to use vsprintf extension %pU [] > > diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c [] > > + sprintf(out->uuid, "%pU", uu); > > } > > EXPORT_SYMBOL(class_uuid_unparse); > > I agree, much better, can you resend this in a form I can apply? Vasiliy, you started this, can you submit this under your name please? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] staging: lustre: obdclass: simplify class_uuid_unparse 2015-07-07 5:01 ` Joe Perches @ 2015-07-15 0:40 ` Vasiliy Korchagin 0 siblings, 0 replies; 7+ messages in thread From: Vasiliy Korchagin @ 2015-07-15 0:40 UTC (permalink / raw) To: oleg.drokin, andreas.dilger Cc: gregkh, vasiliy.korchagin, devel, linux-kernel, joe This patch simplifies uuid unparse logic by using sprintf "%pU" extension. And eliminates the code with a coding style error: ERROR: Macros with complex values should be enclosed in parentheses +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) Signed-off-by: Vasiliy Korchagin <vasiliy.korchagin@gmail.com> --- drivers/staging/lustre/lustre/obdclass/uuid.c | 34 +-------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..b0b0157 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -43,40 +43,8 @@ #include "../include/obd_support.h" #include "../include/obd_class.h" - -static inline __u32 consume(int nob, __u8 **ptr) -{ - __u32 value; - - LASSERT(nob <= sizeof(value)); - - for (value = 0; nob > 0; --nob) - value = (value << 8) | *((*ptr)++); - return value; -} - -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) - -static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) -{ - __u8 *ptr = in; - - LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); - - while (nr-- > 0) - CONSUME(uu[nr], &ptr); -} - void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) { - /* uu as an array of __u16's */ - __u16 uuid[sizeof(class_uuid_t) / sizeof(__u16)]; - - CLASSERT(ARRAY_SIZE(uuid) == 8); - - uuid_unpack(uu, uuid, ARRAY_SIZE(uuid)); - sprintf(out->uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7]); + sprintf(out->uuid, "%pU", uu); } EXPORT_SYMBOL(class_uuid_unparse); -- 2.4.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-07-14 23:41 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-27 4:44 [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c Vasiliy Korchagin 2015-06-27 4:53 ` Joe Perches 2015-06-27 5:36 ` Vasiliy Korchagin 2015-06-27 6:04 ` Joe Perches 2015-07-07 2:36 ` Greg KH 2015-07-07 5:01 ` Joe Perches 2015-07-15 0:40 ` [PATCH] staging: lustre: obdclass: simplify class_uuid_unparse Vasiliy Korchagin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox