* -Wsometimes-uninitialized Clang warning in drivers/tty/serial/qcom_geni_serial.c
@ 2019-03-08 0:45 Nathan Chancellor
2019-03-08 8:40 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2019-03-08 0:45 UTC (permalink / raw)
To: Andy Gross, David Brown, Greg Kroah-Hartman,
Karthikeyan Ramasubramanian, Douglas Anderson, Ryan Case
Cc: linux-arm-msm, linux-serial, linux-kernel, Nick Desaulniers,
clang-built-linux
Hi all,
We are trying to get Clang's -Wsometimes-uninitialized turned on for the
kernel as it can catch some bugs that GCC can't. This warning came up:
drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (options)
^~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1082:37: note: uninitialized use occurs here
return uart_set_options(uport, co, baud, parity, bits, flow);
^~~~
drivers/tty/serial/qcom_geni_serial.c:1079:2: note: remove the 'if' if its condition is always true
if (options)
^~~~~~~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1053:10: note: initialize the variable 'baud' to silence this warning
int baud;
^
= 0
1 warning generated.
While this is probably not an issue in practice (I assume baud is always
supplied as an option), we should clean up this warning. I would fix it
myself but I have no idea what baud's initial value should be as it
seems it is dependent on the driver. Your input would be much
appreciated.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: -Wsometimes-uninitialized Clang warning in drivers/tty/serial/qcom_geni_serial.c
2019-03-08 0:45 -Wsometimes-uninitialized Clang warning in drivers/tty/serial/qcom_geni_serial.c Nathan Chancellor
@ 2019-03-08 8:40 ` Greg Kroah-Hartman
2019-03-08 18:37 ` [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup Nathan Chancellor
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-03-08 8:40 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andy Gross, David Brown, Karthikeyan Ramasubramanian,
Douglas Anderson, Ryan Case, linux-arm-msm, linux-serial,
linux-kernel, Nick Desaulniers, clang-built-linux
On Thu, Mar 07, 2019 at 05:45:26PM -0700, Nathan Chancellor wrote:
> Hi all,
>
> We are trying to get Clang's -Wsometimes-uninitialized turned on for the
> kernel as it can catch some bugs that GCC can't. This warning came up:
>
> drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> if (options)
> ^~~~~~~
> drivers/tty/serial/qcom_geni_serial.c:1082:37: note: uninitialized use occurs here
> return uart_set_options(uport, co, baud, parity, bits, flow);
> ^~~~
> drivers/tty/serial/qcom_geni_serial.c:1079:2: note: remove the 'if' if its condition is always true
> if (options)
> ^~~~~~~~~~~~
> drivers/tty/serial/qcom_geni_serial.c:1053:10: note: initialize the variable 'baud' to silence this warning
> int baud;
> ^
> = 0
> 1 warning generated.
>
> While this is probably not an issue in practice (I assume baud is always
> supplied as an option), we should clean up this warning. I would fix it
> myself but I have no idea what baud's initial value should be as it
> seems it is dependent on the driver. Your input would be much
> appreciated.
this function can be called with the option variable set to NULL, so
there could not be a default baud rate set, nice catch.
As for the default, let's just be sane and set it to 9600 as that's a
normal default baudrate.
Do you want to make up a patch for this, or do you need me to?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup
2019-03-08 8:40 ` Greg Kroah-Hartman
@ 2019-03-08 18:37 ` Nathan Chancellor
2019-03-08 18:40 ` Nick Desaulniers
2019-03-22 14:20 ` Arnd Bergmann
0 siblings, 2 replies; 6+ messages in thread
From: Nathan Chancellor @ 2019-03-08 18:37 UTC (permalink / raw)
To: Andy Gross, David Brown, Greg Kroah-Hartman
Cc: linux-arm-msm, linux-serial, linux-kernel, Nick Desaulniers,
clang-built-linux, Nathan Chancellor
When building with -Wsometimes-uninitialized, Clang warns:
drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud'
is used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
It's not wrong; when options is NULL, baud has no default value. Use
9600 as that is a sane default.
Link: https://github.com/ClangBuiltLinux/linux/issues/395
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/tty/serial/qcom_geni_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3bcec1c20219..35e5f9c5d5be 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1050,7 +1050,7 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
{
struct uart_port *uport;
struct qcom_geni_serial_port *port;
- int baud;
+ int baud = 9600;
int bits = 8;
int parity = 'n';
int flow = 'n';
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup
2019-03-08 18:37 ` [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup Nathan Chancellor
@ 2019-03-08 18:40 ` Nick Desaulniers
2019-03-22 14:20 ` Arnd Bergmann
1 sibling, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-03-08 18:40 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andy Gross, David Brown, Greg Kroah-Hartman, linux-arm-msm,
linux-serial, LKML, clang-built-linux
On Fri, Mar 8, 2019 at 10:38 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud'
> is used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>
> It's not wrong; when options is NULL, baud has no default value. Use
> 9600 as that is a sane default.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/395
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Great find. Thanks for the fix, and thanks Greg for the advice and
additional analysis here.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3bcec1c20219..35e5f9c5d5be 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1050,7 +1050,7 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
> {
> struct uart_port *uport;
> struct qcom_geni_serial_port *port;
> - int baud;
> + int baud = 9600;
> int bits = 8;
> int parity = 'n';
> int flow = 'n';
> --
> 2.21.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup
2019-03-08 18:37 ` [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup Nathan Chancellor
2019-03-08 18:40 ` Nick Desaulniers
@ 2019-03-22 14:20 ` Arnd Bergmann
2019-03-22 18:04 ` Nick Desaulniers
1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:20 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andy Gross, David Brown, Greg Kroah-Hartman, linux-arm-msm,
linux-serial, Linux Kernel Mailing List, Nick Desaulniers,
clang-built-linux
On Fri, Mar 8, 2019 at 7:38 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud'
> is used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>
> It's not wrong; when options is NULL, baud has no default value. Use
> 9600 as that is a sane default.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/395
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3bcec1c20219..35e5f9c5d5be 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1050,7 +1050,7 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
> {
> struct uart_port *uport;
> struct qcom_geni_serial_port *port;
> - int baud;
> + int baud = 9600;
> int bits = 8;
> int parity = 'n';
> int flow = 'n';
I made a similar patch here, but after looking at the driver concluded
that the bitrate had to be 115200, since that is used as the default
value otherwise.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup
2019-03-22 14:20 ` Arnd Bergmann
@ 2019-03-22 18:04 ` Nick Desaulniers
0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-03-22 18:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nathan Chancellor, Andy Gross, David Brown, Greg Kroah-Hartman,
linux-arm-msm, linux-serial, Linux Kernel Mailing List,
clang-built-linux
On Fri, Mar 22, 2019 at 7:20 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Mar 8, 2019 at 7:38 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/tty/serial/qcom_geni_serial.c:1079:6: warning: variable 'baud'
> > is used uninitialized whenever 'if' condition is false
> > [-Wsometimes-uninitialized]
> >
> > It's not wrong; when options is NULL, baud has no default value. Use
> > 9600 as that is a sane default.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/395
> > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > drivers/tty/serial/qcom_geni_serial.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 3bcec1c20219..35e5f9c5d5be 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1050,7 +1050,7 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
> > {
> > struct uart_port *uport;
> > struct qcom_geni_serial_port *port;
> > - int baud;
> > + int baud = 9600;
> > int bits = 8;
> > int parity = 'n';
> > int flow = 'n';
>
> I made a similar patch here, but after looking at the driver concluded
> that the bitrate had to be 115200, since that is used as the default
> value otherwise.
Good point, we use this driver for Pixel, and I usually connect with a
baud rate of 115200.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-22 18:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 0:45 -Wsometimes-uninitialized Clang warning in drivers/tty/serial/qcom_geni_serial.c Nathan Chancellor
2019-03-08 8:40 ` Greg Kroah-Hartman
2019-03-08 18:37 ` [PATCH] tty: serial: qcom_geni_serial: Initialize baud in qcom_geni_console_setup Nathan Chancellor
2019-03-08 18:40 ` Nick Desaulniers
2019-03-22 14:20 ` Arnd Bergmann
2019-03-22 18:04 ` Nick Desaulniers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.