[PATCH] Staging: speakup: Add helper macro for spk_synth boilerplate

[PATCH] Staging: speakup: Add helper macro for spk_synth boilerplate

[Vaishali Thakkar]

For simple modules that contain a single spk_synth without
any additional setup code then ends up being a block of
duplicated boilerplate. This patch adds a new macro,
module_spk_synth(), which replaces the
module_init()/module_exit() registrations with template
functions.

Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
---
v1 : Here, I know, I can go for changing all files
     and make them use module_spk_synth at a single point.
     But as I am sending this patch as a sample patch, I
     am changing only one file to show the use of helper
     macro. I will go for changing all other such cases in
     this driver if this patch will be accepted.

 drivers/staging/speakup/speakup_audptr.c | 12 +-----------
 drivers/staging/speakup/spk_types.h      | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/speakup/speakup_audptr.c b/drivers/staging/speakup/speakup_audptr.c
index 5cbaec8..ea89e36 100644
--- a/drivers/staging/speakup/speakup_audptr.c
+++ b/drivers/staging/speakup/speakup_audptr.c
@@ -177,18 +177,8 @@ module_param_named(start, synth_audptr.startup, short, S_IRUGO);
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
-static int __init audptr_init(void)
-{
-	return synth_add(&synth_audptr);
-}
-
-static void __exit audptr_exit(void)
-{
-	synth_remove(&synth_audptr);
-}
+module_spk_synth(synth_audptr);
 
-module_init(audptr_init);
-module_exit(audptr_exit);
 MODULE_AUTHOR("Kirk Reiser <kirk@braille.uwo.ca>");
 MODULE_AUTHOR("David Borowski");
 MODULE_DESCRIPTION("Speakup support for Audapter synthesizer");
diff --git a/drivers/staging/speakup/spk_types.h b/drivers/staging/speakup/spk_types.h
index 8c565c9..e0c4fe3 100644
--- a/drivers/staging/speakup/spk_types.h
+++ b/drivers/staging/speakup/spk_types.h
@@ -179,6 +179,23 @@ struct spk_synth {
 	struct attribute_group attributes;
 };
 
+/* module_spk_synth() - Helper macro for speakup drivers that don't
+ * do anything special in module init/exit. This eliminates a lot
+ * of boilerplate. Each module may only use this macro once, and
+ * calling it replaces module_init() and module_exit()
+ */
+#define module_spk_synth(__spk_synth) \
+static int __init __spk_synth##_init(void) \
+{ \
+	return synth_add(&(__spk_synth)); \
+} \
+module_init(__spk_synth##_init); \
+static void __exit __spk_synth##_exit(void) \
+{ \
+	synth_remove(&(__spk_synth)); \
+} \
+module_exit(__spk_synth##_exit);
+
 struct speakup_info_t {
 	spinlock_t spinlock;
 	int port_tts;
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: speakup: Add helper macro for spk_synth boilerplate

[Greg KH]

On Sat, Mar 14, 2015 at 10:22:35PM +0530, Vaishali Thakkar wrote:
> For simple modules that contain a single spk_synth without
> any additional setup code then ends up being a block of
> duplicated boilerplate. This patch adds a new macro,
> module_spk_synth(), which replaces the
> module_init()/module_exit() registrations with template
> functions.
> 
> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> ---
> v1 : Here, I know, I can go for changing all files
>      and make them use module_spk_synth at a single point.
>      But as I am sending this patch as a sample patch, I
>      am changing only one file to show the use of helper
>      macro. I will go for changing all other such cases in
>      this driver if this patch will be accepted.
> 
>  drivers/staging/speakup/speakup_audptr.c | 12 +-----------
>  drivers/staging/speakup/spk_types.h      | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/speakup/speakup_audptr.c b/drivers/staging/speakup/speakup_audptr.c
> index 5cbaec8..ea89e36 100644
> --- a/drivers/staging/speakup/speakup_audptr.c
> +++ b/drivers/staging/speakup/speakup_audptr.c
> @@ -177,18 +177,8 @@ module_param_named(start, synth_audptr.startup, short, S_IRUGO);
>  MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
>  MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
>  
> -static int __init audptr_init(void)
> -{
> -	return synth_add(&synth_audptr);
> -}
> -
> -static void __exit audptr_exit(void)
> -{
> -	synth_remove(&synth_audptr);
> -}
> +module_spk_synth(synth_audptr);
>  
> -module_init(audptr_init);
> -module_exit(audptr_exit);
>  MODULE_AUTHOR("Kirk Reiser <kirk@braille.uwo.ca>");
>  MODULE_AUTHOR("David Borowski");
>  MODULE_DESCRIPTION("Speakup support for Audapter synthesizer");
> diff --git a/drivers/staging/speakup/spk_types.h b/drivers/staging/speakup/spk_types.h
> index 8c565c9..e0c4fe3 100644
> --- a/drivers/staging/speakup/spk_types.h
> +++ b/drivers/staging/speakup/spk_types.h
> @@ -179,6 +179,23 @@ struct spk_synth {
>  	struct attribute_group attributes;
>  };
>  
> +/* module_spk_synth() - Helper macro for speakup drivers that don't
> + * do anything special in module init/exit. This eliminates a lot
> + * of boilerplate. Each module may only use this macro once, and
> + * calling it replaces module_init() and module_exit()
> + */
> +#define module_spk_synth(__spk_synth) \
> +static int __init __spk_synth##_init(void) \
> +{ \
> +	return synth_add(&(__spk_synth)); \
> +} \
> +module_init(__spk_synth##_init); \
> +static void __exit __spk_synth##_exit(void) \
> +{ \
> +	synth_remove(&(__spk_synth)); \
> +} \
> +module_exit(__spk_synth##_exit);
> +

Can't you use the module_driver() macro here instead of "rolling your
own"?  Look at how module_usb_driver() is defined as an example.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH] Staging: speakup: Add helper macro for spk_synth boilerplate

[Vaishali Thakkar]

On Sun, Mar 15, 2015 at 4:20 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Mar 14, 2015 at 10:22:35PM +0530, Vaishali Thakkar wrote:
>> For simple modules that contain a single spk_synth without
>> any additional setup code then ends up being a block of
>> duplicated boilerplate. This patch adds a new macro,
>> module_spk_synth(), which replaces the
>> module_init()/module_exit() registrations with template
>> functions.
>>
>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
>> ---
>> v1 : Here, I know, I can go for changing all files
>>      and make them use module_spk_synth at a single point.
>>      But as I am sending this patch as a sample patch, I
>>      am changing only one file to show the use of helper
>>      macro. I will go for changing all other such cases in
>>      this driver if this patch will be accepted.
>>
>>  drivers/staging/speakup/speakup_audptr.c | 12 +-----------
>>  drivers/staging/speakup/spk_types.h      | 17 +++++++++++++++++
>>  2 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/speakup/speakup_audptr.c b/drivers/staging/speakup/speakup_audptr.c
>> index 5cbaec8..ea89e36 100644
>> --- a/drivers/staging/speakup/speakup_audptr.c
>> +++ b/drivers/staging/speakup/speakup_audptr.c
>> @@ -177,18 +177,8 @@ module_param_named(start, synth_audptr.startup, short, S_IRUGO);
>>  MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
>>  MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
>>
>> -static int __init audptr_init(void)
>> -{
>> -     return synth_add(&synth_audptr);
>> -}
>> -
>> -static void __exit audptr_exit(void)
>> -{
>> -     synth_remove(&synth_audptr);
>> -}
>> +module_spk_synth(synth_audptr);
>>
>> -module_init(audptr_init);
>> -module_exit(audptr_exit);
>>  MODULE_AUTHOR("Kirk Reiser <kirk@braille.uwo.ca>");
>>  MODULE_AUTHOR("David Borowski");
>>  MODULE_DESCRIPTION("Speakup support for Audapter synthesizer");
>> diff --git a/drivers/staging/speakup/spk_types.h b/drivers/staging/speakup/spk_types.h
>> index 8c565c9..e0c4fe3 100644
>> --- a/drivers/staging/speakup/spk_types.h
>> +++ b/drivers/staging/speakup/spk_types.h
>> @@ -179,6 +179,23 @@ struct spk_synth {
>>       struct attribute_group attributes;
>>  };
>>
>> +/* module_spk_synth() - Helper macro for speakup drivers that don't
>> + * do anything special in module init/exit. This eliminates a lot
>> + * of boilerplate. Each module may only use this macro once, and
>> + * calling it replaces module_init() and module_exit()
>> + */
>> +#define module_spk_synth(__spk_synth) \
>> +static int __init __spk_synth##_init(void) \
>> +{ \
>> +     return synth_add(&(__spk_synth)); \
>> +} \
>> +module_init(__spk_synth##_init); \
>> +static void __exit __spk_synth##_exit(void) \
>> +{ \
>> +     synth_remove(&(__spk_synth)); \
>> +} \
>> +module_exit(__spk_synth##_exit);
>> +
>
> Can't you use the module_driver() macro here instead of "rolling your
> own"?  Look at how module_usb_driver() is defined as an example.

Yes. I guess here I can use module_driver(). I will send revised version
with this change.

Thank You.

> thanks,
>
> greg k-h



-- 
Vaishali