[PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[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 <bart.vanassche@gmail.com>

diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
index 18a5b9b..61cc908 100644
--- a/include/linux/ratelimit.h
+++ b/include/linux/ratelimit.h
@@ -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);
 

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

Re: [PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[Bart Van Assche]

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.

On Tue, Nov 4, 2008 at 9:14 PM, Bart Van Assche
<bart.vanassche@gmail.com> wrote:
> 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 <bart.vanassche@gmail.com>
>
> diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
> index 18a5b9b..61cc908 100644
> --- a/include/linux/ratelimit.h
> +++ b/include/linux/ratelimit.h
> @@ -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);
>

Re: [PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[Randy Dunlap]

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.
> 
> On Tue, Nov 4, 2008 at 9:14 PM, Bart Van Assche
> <bart.vanassche@gmail.com> wrote:
> > 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 <bart.vanassche@gmail.com>
> >
> > diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
> > index 18a5b9b..61cc908 100644
> > --- a/include/linux/ratelimit.h
> > +++ b/include/linux/ratelimit.h
> > @@ -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);

I thought that Andrew would pick it up.

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

Anyway,
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Thanks,
---
~Randy

Re: [PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[Andrew Morton]

On Mon, 10 Nov 2008 09:16:02 -0800 Randy Dunlap <randy.dunlap@oracle.com> wrote:

> 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.
> > 
> > On Tue, Nov 4, 2008 at 9:14 PM, Bart Van Assche
> > <bart.vanassche@gmail.com> wrote:
> > > 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 <bart.vanassche@gmail.com>
> > >
> > > diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
> > > index 18a5b9b..61cc908 100644
> > > --- a/include/linux/ratelimit.h
> > > +++ b/include/linux/ratelimit.h
> > > @@ -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);
> 
> 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.

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

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

Re: [PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[Linus Torvalds]

On Mon, 10 Nov 2008, Andrew Morton wrote:
> 
> 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

Re: [PATCH][RESEND] C99 initializers for DEFINE_RATELIMIT_STATE()

[Bart Van Assche]

On Mon, Nov 10, 2008 at 6:34 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, 10 Nov 2008, Andrew Morton wrote:
>>
>> 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.