C99 initializers for DEFINE_RATELIMIT_STATE()

PATCH RESEND In review Bart Van Assche 4 participants 6 messages Nov 4, 2008 – Nov 10, 2008

Share & tools

Bart Van Assche

author

2008-11-04 20:15 UTC #0 lore raw

PATCH RESEND 1/1 C99 initializers for DEFINE_RATELIMIT_STATE()

A few months ago (July 25, 2008) the printk ratelimiting code has been rewritten. This patch added a.o. the new macro DEFINE_RATELIMIT_STATE(). The patch below converts the initializers in that macro to C99 style.

For more information about the printk ratelimiting rewrite, see also commit 717115e1a5856b57af0f71e1df7149108294fc10
(http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.27.y.git;a=commit;h=717115e1a5856b57af0f71e1df7149108294fc10).

Impact: cleanup, no functionality changed.

Signed-off-by Bart Van Assche <[email protected]>

include/linux/ratelimit.h +3 -2

index 18a5b9b..61cc908 100644
@@ -13,8 +13,9 @@ struct ratelimit_state {
 	unsigned long begin;
 };
 
-#define DEFINE_RATELIMIT_STATE(name, interval, burst)		\
-		struct ratelimit_state name = {interval, burst,}
+#define DEFINE_RATELIMIT_STATE(name, interval_value, burst_value)	\
+	struct ratelimit_state name =					\
+		{ .interval = (interval_value), .burst = (burst_value) }
 
 extern int __ratelimit(struct ratelimit_state *rs);

-------------------------------------------------------

Bart Van Assche

author

2008-11-10 12:43 UTC #1 lore raw

Could someone please be so kind to ACK the patch below, NAK it or tell me whether I should send the patch below to someone else ?

Thanks,

Bart.

Quoted 28 lines · Bart Van Assche

A few months ago (July 25, 2008) the printk ratelimiting code has been rewritten. This patch added a.o. the new macro DEFINE_RATELIMIT_STATE(). The patch below converts the initializers in that macro to C99 style.
For more information about the printk ratelimiting rewrite, see also commit 717115e1a5856b57af0f71e1df7149108294fc10
(http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.27.y.git;a=commit;h=717115e1a5856b57af0f71e1df7149108294fc10).
Impact: cleanup, no functionality changed.
Signed-off-by Bart Van Assche <[email protected]>
include/linux/ratelimit.h +3 -2
index 18a5b9b..61cc908 100644
@@ -13,8 +13,9 @@ struct ratelimit_state {
       unsigned long begin;
 };
 
-#define DEFINE_RATELIMIT_STATE(name, interval, burst)          \
-               struct ratelimit_state name = {interval, burst,}
+#define DEFINE_RATELIMIT_STATE(name, interval_value, burst_value)      \
+       struct ratelimit_state name =                                   \
+               { .interval = (interval_value), .burst = (burst_value) }
 
 extern int __ratelimit(struct ratelimit_state *rs);
 

Randy Dunlap

Acked-by

2008-11-10 17:17 UTC #2 lore raw

Quoted 29 lines · On Mon, 10 Nov 2008 13:43:24 +0100 Bart Van Assche wrote:

Could someone please be so kind to ACK the patch below, NAK it or tell me whether I should send the patch below to someone else ?
Thanks,
Bart.
Quoted 20 lines · Bart Van Assche
A few months ago (July 25, 2008) the printk ratelimiting code has been rewritten. This patch added a.o. the new macro DEFINE_RATELIMIT_STATE(). The patch below converts the initializers in that macro to C99 style.
For more information about the printk ratelimiting rewrite, see also commit 717115e1a5856b57af0f71e1df7149108294fc10
(http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.27.y.git;a=commit;h=717115e1a5856b57af0f71e1df7149108294fc10).
Impact: cleanup, no functionality changed.
Signed-off-by Bart Van Assche <[email protected]>
include/linux/ratelimit.h +0 -0
index 18a5b9b..61cc908 100644
@@ -13,8 +13,9 @@ struct ratelimit_state {
       unsigned long begin;
 };
 

Quoted 7 lines

include/linux/ratelimit.h struct ratelimit_state { · line 20

Quoted 7 lines

include/linux/ratelimit.h +3 -2

-#define DEFINE_RATELIMIT_STATE(name, interval, burst)          \
-               struct ratelimit_state name = {interval, burst,}
+#define DEFINE_RATELIMIT_STATE(name, interval_value, burst_value)      \
+       struct ratelimit_state name =                                   \
+               { .interval = (interval_value), .burst = (burst_value) }
 
 extern int __ratelimit(struct ratelimit_state *rs);

I thought that Andrew would pick it up.

Nit: I would put the .interval = ... and .burst = ... on separate lines.

Anyway,

Acked-by Randy Dunlap <[email protected]>

Thanks,
---
~Randy

Andrew Morton

2008-11-10 17:31 UTC #3 lore raw

Quoted 37 lines · On Mon, 10 Nov 2008 09:16:02 -0800 Randy Dunlap <[email protected]> wrote:

Quoted 35 lines · On Mon, 10 Nov 2008 13:43:24 +0100 Bart Van Assche wrote:
Could someone please be so kind to ACK the patch below, NAK it or tell me whether I should send the patch below to someone else ?
Thanks,
Bart.
Quoted 26 lines · Bart Van Assche
A few months ago (July 25, 2008) the printk ratelimiting code has been rewritten. This patch added a.o. the new macro DEFINE_RATELIMIT_STATE(). The patch below converts the initializers in that macro to C99 style.
For more information about the printk ratelimiting rewrite, see also commit 717115e1a5856b57af0f71e1df7149108294fc10
(http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.27.y.git;a=commit;h=717115e1a5856b57af0f71e1df7149108294fc10).
Impact: cleanup, no functionality changed.
Signed-off-by Bart Van Assche <[email protected]>
include/linux/ratelimit.h +3 -2
index 18a5b9b..61cc908 100644
@@ -13,8 +13,9 @@ struct ratelimit_state {
       unsigned long begin;
 };
 
-#define DEFINE_RATELIMIT_STATE(name, interval, burst)          \
-               struct ratelimit_state name = {interval, burst,}
+#define DEFINE_RATELIMIT_STATE(name, interval_value, burst_value)      \
+       struct ratelimit_state name =                                   \
+               { .interval = (interval_value), .burst = (burst_value) }
 

Quoted 3 lines

Quoted 1 line
include/linux/ratelimit.h struct ratelimit_state { · line 20
Quoted 1 line
include/linux/ratelimit.h +0 -0
 extern int __ratelimit(struct ratelimit_state *rs);
I thought that Andrew would pick it up.

I already have a patch queued which does this (linux-ratelimith-fixed-missing-initializer-warning.patch). I sent it to Linus on October 29 and he did not apply it, so I parked it for 2.6.29.

Quoted 1 line

Nit: I would put the .interval = ... and .burst = ... on separate lines.

linux-ratelimith-fixed-missing-initializer-warning.patch does it that way.

Linus Torvalds

2008-11-10 17:34 UTC #4 lore raw

Quoted 4 lines · Andrew Morton

I already have a patch queued which does this (linux-ratelimith-fixed-missing-initializer-warning.patch). I sent it to Linus on October 29 and he did not apply it, so I parked it for 2.6.29.

I just don't see the point of the patch at all. It doesn't fix anything, and it just makes the code bigger and uglier. We have tons of other simple initializers that aren't C99, and we're not going to convert them either.

Linus

Bart Van Assche

author

2008-11-10 18:07 UTC #5 lore raw

Quoted 10 lines · Linus Torvalds

Quoted 4 lines · Andrew Morton
I already have a patch queued which does this (linux-ratelimith-fixed-missing-initializer-warning.patch). I sent it to Linus on October 29 and he did not apply it, so I parked it for 2.6.29.
I just don't see the point of the patch at all. It doesn't fix anything, and it just makes the code bigger and uglier. We have tons of other simple initializers that aren't C99, and we're not going to convert them either.

The current version of the header file include/linux/ratelimit.h triggers false positives when the compiler flag -Wmissing-field-initializers is specified. The header file include/linux/ratelimit.h is included directly or indirectly by many other header files -- inclusion of this file can't be avoided. The above patch suppresses these false positives by converting the initializers to C99 style, such that kernel developers who want to check their code through -Wmissing-field-initializers can use this compiler flag. And I do not have plans to send patches for converting initializers to C99-style for any .c file.

Bart.