2018-07-03 07:35:00

by Tamir Suliman

[permalink] [raw]
Subject: [PATCH 3/3] drivers/speakup: Fix style and coding warnings

Fixed the following style/coding issues:
*updated ---help to the prefered new help texts which reduces the code/file size and fixes the warning messages
*Used else if instead of elese as else is not generally useful after a break or return, not sure if this is the acceptable but it resolved the warning messages.
*kstrtoul is used instead of the abosolete simple_strtoul

Signed-off-by: Tamir Suliman <[email protected]>
---
drivers/staging/speakup/Kconfig | 43 ++++++++++++++------------------------
drivers/staging/speakup/keyhelp.c | 2 +-
drivers/staging/speakup/kobjects.c | 4 ++--
3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/speakup/Kconfig b/drivers/staging/speakup/Kconfig
index efd6f45..2f282fd 100644
--- a/drivers/staging/speakup/Kconfig
+++ b/drivers/staging/speakup/Kconfig
@@ -3,7 +3,7 @@ menu "Speakup console speech"
config SPEAKUP
depends on VT
tristate "Speakup core"
- ---help---
+ help
This is the Speakup screen reader. Think of it as a
video console for blind people. If built in to the
kernel, it can speak everything on the text console from
@@ -43,7 +43,7 @@ config SPEAKUP
if SPEAKUP
config SPEAKUP_SYNTH_ACNTSA
tristate "Accent SA synthesizer support"
- ---help---
+ help
This is the Speakup driver for the Accent SA
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -52,7 +52,7 @@ config SPEAKUP_SYNTH_ACNTSA
config SPEAKUP_SYNTH_ACNTPC
tristate "Accent PC synthesizer support"
depends on ISA || COMPILE_TEST
- ---help---
+ help
This is the Speakup driver for the accent pc
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -60,7 +60,7 @@ config SPEAKUP_SYNTH_ACNTPC

config SPEAKUP_SYNTH_APOLLO
tristate "Apollo II synthesizer support"
- ---help---
+ help
This is the Speakup driver for the Apollo II
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -68,7 +68,7 @@ config SPEAKUP_SYNTH_APOLLO

config SPEAKUP_SYNTH_AUDPTR
tristate "Audapter synthesizer support"
- ---help---
+ help
This is the Speakup driver for the Audapter synthesizer.
You can say y to build it into the kernel, or m to
build it as a module. See the configuration help on the
@@ -76,7 +76,7 @@ config SPEAKUP_SYNTH_AUDPTR

config SPEAKUP_SYNTH_BNS
tristate "Braille 'n' Speak synthesizer support"
- ---help---
+ help
This is the Speakup driver for the Braille 'n' Speak
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -84,8 +84,7 @@ config SPEAKUP_SYNTH_BNS

config SPEAKUP_SYNTH_DECTLK
tristate "DECtalk Express synthesizer support"
- ---help---
-
+ help
This is the Speakup driver for the DecTalk Express
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -93,8 +92,7 @@ config SPEAKUP_SYNTH_DECTLK

config SPEAKUP_SYNTH_DECEXT
tristate "DECtalk External (old) synthesizer support"
- ---help---
-
+ help
This is the Speakup driver for the DecTalk External
(old) synthesizer. You can say y to build it into the
kernel, or m to build it as a module. See the
@@ -105,13 +103,11 @@ config SPEAKUP_SYNTH_DECPC
depends on m
depends on ISA || COMPILE_TEST
tristate "DECtalk PC (big ISA card) synthesizer support"
- ---help---
-
+ help
This is the Speakup driver for the DecTalk PC (full
length ISA) synthesizer. You can say m to build it as
a module. See the configuration help on the Speakup
choice above for more info.
-
In order to use the DecTalk PC driver, you must download
the dec_pc.tgz file from linux-speakup.org. It is in
the pub/linux/goodies directory. The dec_pc.tgz file
@@ -127,8 +123,7 @@ config SPEAKUP_SYNTH_DECPC
config SPEAKUP_SYNTH_DTLK
tristate "DoubleTalk PC synthesizer support"
depends on ISA || COMPILE_TEST
- ---help---
-
+ help
This is the Speakup driver for the internal DoubleTalk
PC synthesizer. You can say y to build it into the
kernel, or m to build it as a module. See the
@@ -138,8 +133,7 @@ config SPEAKUP_SYNTH_DTLK
config SPEAKUP_SYNTH_KEYPC
tristate "Keynote Gold PC synthesizer support"
depends on ISA || COMPILE_TEST
- ---help---
-
+ help
This is the Speakup driver for the Keynote Gold
PC synthesizer. You can say y to build it into the
kernel, or m to build it as a module. See the
@@ -148,8 +142,7 @@ config SPEAKUP_SYNTH_KEYPC

config SPEAKUP_SYNTH_LTLK
tristate "DoubleTalk LT/LiteTalk synthesizer support"
----help---
-
+ help
This is the Speakup driver for the LiteTalk/DoubleTalk
LT synthesizer. You can say y to build it into the
kernel, or m to build it as a module. See the
@@ -158,8 +151,7 @@ config SPEAKUP_SYNTH_LTLK

config SPEAKUP_SYNTH_SOFT
tristate "Userspace software synthesizer support"
- ---help---
-
+ help
This is the software synthesizer device node. It will
register a device /dev/softsynth which midware programs
and speech daemons may open and read to provide kernel
@@ -169,8 +161,7 @@ config SPEAKUP_SYNTH_SOFT

config SPEAKUP_SYNTH_SPKOUT
tristate "Speak Out synthesizer support"
- ---help---
-
+ help
This is the Speakup driver for the Speakout synthesizer.
You can say y to build it into the kernel, or m to
build it as a module. See the configuration help on the
@@ -178,8 +169,7 @@ config SPEAKUP_SYNTH_SPKOUT

config SPEAKUP_SYNTH_TXPRT
tristate "Transport synthesizer support"
- ---help---
-
+ help
This is the Speakup driver for the Transport
synthesizer. You can say y to build it into the kernel,
or m to build it as a module. See the configuration
@@ -187,8 +177,7 @@ config SPEAKUP_SYNTH_TXPRT

config SPEAKUP_SYNTH_DUMMY
tristate "Dummy synthesizer driver (for testing)"
- ---help---
-
+ help
This is a dummy Speakup driver for plugging a mere serial
terminal. This is handy if you want to test speakup but
don't have the hardware. You can say y to build it into
diff --git a/drivers/staging/speakup/keyhelp.c b/drivers/staging/speakup/keyhelp.c
index 5f1bda3..a1bbe8f 100644
--- a/drivers/staging/speakup/keyhelp.c
+++ b/drivers/staging/speakup/keyhelp.c
@@ -167,7 +167,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
build_key_data(); /* rebuild each time in case new mapping */
return 1;
- } else {
+ } else if {
name = NULL;
if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
synth_printf("%s\n",
diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index f1f9022..a98a0a4 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -154,7 +154,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
continue;
}

- index = simple_strtoul(cp, &temp, 10);
+ index = simple_kstrtoul(cp, &temp, 10);
if (index > 255) {
rejected++;
cp = linefeed + 1;
@@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
continue;
}

- index = simple_strtoul(cp, &temp, 10);
+ index = simple_ktrtoul(cp, &temp, 10);

while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
temp++;
--
1.8.3.1



2018-07-03 07:41:19

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings

On Tue, Jul 03, 2018 at 07:31:20AM +0000, Tamir Suliman wrote:
> Fixed the following style/coding issues:
> *updated ---help to the prefered new help texts which reduces the code/file size and fixes the warning messages
> *Used else if instead of elese as else is not generally useful after a break or return, not sure if this is the acceptable but it resolved the warning messages.
> *kstrtoul is used instead of the abosolete simple_strtoul
>
> Signed-off-by: Tamir Suliman <[email protected]>
> ---
> drivers/staging/speakup/Kconfig | 43 ++++++++++++++------------------------
> drivers/staging/speakup/keyhelp.c | 2 +-
> drivers/staging/speakup/kobjects.c | 4 ++--
> 3 files changed, 19 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/staging/speakup/Kconfig b/drivers/staging/speakup/Kconfig
> index efd6f45..2f282fd 100644
> --- a/drivers/staging/speakup/Kconfig
> +++ b/drivers/staging/speakup/Kconfig
> @@ -3,7 +3,7 @@ menu "Speakup console speech"
> config SPEAKUP
> depends on VT
> tristate "Speakup core"
> - ---help---
> + help
> This is the Speakup screen reader. Think of it as a
> video console for blind people. If built in to the
> kernel, it can speak everything on the text console from
> @@ -43,7 +43,7 @@ config SPEAKUP
> if SPEAKUP
> config SPEAKUP_SYNTH_ACNTSA
> tristate "Accent SA synthesizer support"
> - ---help---
> + help
> This is the Speakup driver for the Accent SA
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -52,7 +52,7 @@ config SPEAKUP_SYNTH_ACNTSA
> config SPEAKUP_SYNTH_ACNTPC
> tristate "Accent PC synthesizer support"
> depends on ISA || COMPILE_TEST
> - ---help---
> + help
> This is the Speakup driver for the accent pc
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -60,7 +60,7 @@ config SPEAKUP_SYNTH_ACNTPC
>
> config SPEAKUP_SYNTH_APOLLO
> tristate "Apollo II synthesizer support"
> - ---help---
> + help
> This is the Speakup driver for the Apollo II
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -68,7 +68,7 @@ config SPEAKUP_SYNTH_APOLLO
>
> config SPEAKUP_SYNTH_AUDPTR
> tristate "Audapter synthesizer support"
> - ---help---
> + help
> This is the Speakup driver for the Audapter synthesizer.
> You can say y to build it into the kernel, or m to
> build it as a module. See the configuration help on the
> @@ -76,7 +76,7 @@ config SPEAKUP_SYNTH_AUDPTR
>
> config SPEAKUP_SYNTH_BNS
> tristate "Braille 'n' Speak synthesizer support"
> - ---help---
> + help
> This is the Speakup driver for the Braille 'n' Speak
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -84,8 +84,7 @@ config SPEAKUP_SYNTH_BNS
>
> config SPEAKUP_SYNTH_DECTLK
> tristate "DECtalk Express synthesizer support"
> - ---help---
> -
> + help
> This is the Speakup driver for the DecTalk Express
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -93,8 +92,7 @@ config SPEAKUP_SYNTH_DECTLK
>
> config SPEAKUP_SYNTH_DECEXT
> tristate "DECtalk External (old) synthesizer support"
> - ---help---
> -
> + help
> This is the Speakup driver for the DecTalk External
> (old) synthesizer. You can say y to build it into the
> kernel, or m to build it as a module. See the
> @@ -105,13 +103,11 @@ config SPEAKUP_SYNTH_DECPC
> depends on m
> depends on ISA || COMPILE_TEST
> tristate "DECtalk PC (big ISA card) synthesizer support"
> - ---help---
> -
> + help
> This is the Speakup driver for the DecTalk PC (full
> length ISA) synthesizer. You can say m to build it as
> a module. See the configuration help on the Speakup
> choice above for more info.
> -
> In order to use the DecTalk PC driver, you must download
> the dec_pc.tgz file from linux-speakup.org. It is in
> the pub/linux/goodies directory. The dec_pc.tgz file
> @@ -127,8 +123,7 @@ config SPEAKUP_SYNTH_DECPC
> config SPEAKUP_SYNTH_DTLK
> tristate "DoubleTalk PC synthesizer support"
> depends on ISA || COMPILE_TEST
> - ---help---
> -
> + help
> This is the Speakup driver for the internal DoubleTalk
> PC synthesizer. You can say y to build it into the
> kernel, or m to build it as a module. See the
> @@ -138,8 +133,7 @@ config SPEAKUP_SYNTH_DTLK
> config SPEAKUP_SYNTH_KEYPC
> tristate "Keynote Gold PC synthesizer support"
> depends on ISA || COMPILE_TEST
> - ---help---
> -
> + help
> This is the Speakup driver for the Keynote Gold
> PC synthesizer. You can say y to build it into the
> kernel, or m to build it as a module. See the
> @@ -148,8 +142,7 @@ config SPEAKUP_SYNTH_KEYPC
>
> config SPEAKUP_SYNTH_LTLK
> tristate "DoubleTalk LT/LiteTalk synthesizer support"
> ----help---
> -
> + help
> This is the Speakup driver for the LiteTalk/DoubleTalk
> LT synthesizer. You can say y to build it into the
> kernel, or m to build it as a module. See the
> @@ -158,8 +151,7 @@ config SPEAKUP_SYNTH_LTLK
>
> config SPEAKUP_SYNTH_SOFT
> tristate "Userspace software synthesizer support"
> - ---help---
> -
> + help
> This is the software synthesizer device node. It will
> register a device /dev/softsynth which midware programs
> and speech daemons may open and read to provide kernel
> @@ -169,8 +161,7 @@ config SPEAKUP_SYNTH_SOFT
>
> config SPEAKUP_SYNTH_SPKOUT
> tristate "Speak Out synthesizer support"
> - ---help---
> -
> + help
> This is the Speakup driver for the Speakout synthesizer.
> You can say y to build it into the kernel, or m to
> build it as a module. See the configuration help on the
> @@ -178,8 +169,7 @@ config SPEAKUP_SYNTH_SPKOUT
>
> config SPEAKUP_SYNTH_TXPRT
> tristate "Transport synthesizer support"
> - ---help---
> -
> + help
> This is the Speakup driver for the Transport
> synthesizer. You can say y to build it into the kernel,
> or m to build it as a module. See the configuration
> @@ -187,8 +177,7 @@ config SPEAKUP_SYNTH_TXPRT
>
> config SPEAKUP_SYNTH_DUMMY
> tristate "Dummy synthesizer driver (for testing)"
> - ---help---
> -
> + help
> This is a dummy Speakup driver for plugging a mere serial
> terminal. This is handy if you want to test speakup but
> don't have the hardware. You can say y to build it into
> diff --git a/drivers/staging/speakup/keyhelp.c b/drivers/staging/speakup/keyhelp.c
> index 5f1bda3..a1bbe8f 100644
> --- a/drivers/staging/speakup/keyhelp.c
> +++ b/drivers/staging/speakup/keyhelp.c
> @@ -167,7 +167,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
> synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
> build_key_data(); /* rebuild each time in case new mapping */
> return 1;
> - } else {
> + } else if {
> name = NULL;
> if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
> synth_printf("%s\n",
> diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
> index f1f9022..a98a0a4 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -154,7 +154,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
> continue;
> }
>
> - index = simple_strtoul(cp, &temp, 10);
> + index = simple_kstrtoul(cp, &temp, 10);
> if (index > 255) {
> rejected++;
> cp = linefeed + 1;
> @@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
> continue;
> }
>
> - index = simple_strtoul(cp, &temp, 10);
> + index = simple_ktrtoul(cp, &temp, 10);
>
> while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
> temp++;
> --
> 1.8.3.1

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
to review. All Linux kernel patches need to only do one thing at a
time. If you need to do multiple things (such as clean up all coding
style issues in a file/driver), do it in a sequence of patches, each
one doing only one thing. This will make it easier to review the
patches to ensure that they are correct, and to help alleviate any
merge issues that larger patches can cause.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2018-07-03 08:13:12

by Justin Skists

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings


> On 03 July 2018 at 08:31 Tamir Suliman <[email protected]> wrote:

> +++ b/drivers/staging/speakup/keyhelp.c
> @@ -167,7 +167,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
> synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
> build_key_data(); /* rebuild each time in case new mapping */
> return 1;
> - } else {
> + } else if {

Interesting construct...


> @@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
> continue;
> }
>
> - index = simple_strtoul(cp, &temp, 10);
> + index = simple_ktrtoul(cp, &temp, 10);

Did you compile this?


Justin.

2018-07-04 19:50:51

by Tamir Suliman

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings


Interesting construct...  Yeah I'm little bit rusty on my C
/programming  . I understand  proper coding style may be should end with
else so I wasn't sure .. however this resolved the warnings. :)


Did you compile this?


Yes I did compile  however the only issue i found when I'm compiling is 
modules symvers missing messages researched online and found couple of
articles that recommended  to  run full kernel build . Please bear with
me I'm new to this and very excited about learning and contributing.



On 7/3/2018 11:10 AM, Justin Skists wrote:
>> On 03 July 2018 at 08:31 Tamir Suliman <[email protected]> wrote:
>> +++ b/drivers/staging/speakup/keyhelp.c
>> @@ -167,7 +167,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
>> synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
>> build_key_data(); /* rebuild each time in case new mapping */
>> return 1;
>> - } else {
>> + } else if {
> Interesting construct...
>
>
>> @@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
>> continue;
>> }
>>
>> - index = simple_strtoul(cp, &temp, 10);
>> + index = simple_ktrtoul(cp, &temp, 10);
> Did you compile this?
>
>
> Justin.


2018-07-05 11:49:11

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings



On Wed, Jul 04, 2018 at 10:46:05PM +0300, Tamir Suliman wrote:
>
> Interesting construct...? Yeah I'm little bit rusty on my C /programming? .
> I understand? proper coding style may be should end with else so I wasn't
> sure .. however this resolved the warnings. :)


The code you wrote was very wrong.

It doesn't compile. You need to figure out what is wrong with your
testing so that it doesn't happen again.

regards,
dan carpenter


2018-07-05 11:58:04

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings

On Tue, Jul 03, 2018 at 09:10:55AM +0100, Justin Skists wrote:
> > @@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
> > continue;
> > }
> >
> > - index = simple_strtoul(cp, &temp, 10);
> > + index = simple_ktrtoul(cp, &temp, 10);
>
> Did you compile this?

Try to avoid rhetorical questions. Just say "This doesn't compile. Be
more careful."

regards,
dan carpenter