gcc-10 points out an uninitialized variable use:
drivers/staging/greybus/uart.c: In function 'gb_tty_set_termios': drivers/staging/greybus/uart.c:540:24: error: 'newline.flow_control' is used uninitialized in this function [-Werror=uninitialized] 540 | newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN;
Instead of using |= and &= on the uninitialized variable, use a direct assignment.
Fixes: e55c25206d5c ("greybus: uart: Handle CRTSCTS flag in termios") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/staging/greybus/uart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 55c51143bb09..4ffb334cd5cd 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -537,9 +537,9 @@ static void gb_tty_set_termios(struct tty_struct *tty, }
if (C_CRTSCTS(tty) && C_BAUD(tty) != B0) - newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN; + newline.flow_control = GB_SERIAL_AUTO_RTSCTS_EN; else - newline.flow_control &= ~GB_SERIAL_AUTO_RTSCTS_EN; + newline.flow_control = 0;
if (memcmp(&gb_tty->line_coding, &newline, sizeof(newline))) { memcpy(&gb_tty->line_coding, &newline, sizeof(newline));
On 4/29/20 2:00 PM, Arnd Bergmann wrote:
gcc-10 points out an uninitialized variable use:
Wow, nice, checking individual uninitialized fields within the structure.
The structure should really be zero-initialized anyway; it's passed as a structure in a message elsewhere. With your change, all fields in the structure are written, but in theory the structure could change and stack garbage could be sent over the wire.
What do you think of doing this instead? Or in addition?
struct gb_tty_line_coding newline = { };
(Presumably that would also silence the warning.)
I endorse of your change, either way.
-Alex
drivers/staging/greybus/uart.c: In function 'gb_tty_set_termios': drivers/staging/greybus/uart.c:540:24: error: 'newline.flow_control' is used uninitialized in this function [-Werror=uninitialized] 540 | newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN;
Instead of using |= and &= on the uninitialized variable, use a direct assignment.
Fixes: e55c25206d5c ("greybus: uart: Handle CRTSCTS flag in termios") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/staging/greybus/uart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 55c51143bb09..4ffb334cd5cd 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -537,9 +537,9 @@ static void gb_tty_set_termios(struct tty_struct *tty, } if (C_CRTSCTS(tty) && C_BAUD(tty) != B0)
newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN;
elsenewline.flow_control = GB_SERIAL_AUTO_RTSCTS_EN;
newline.flow_control &= ~GB_SERIAL_AUTO_RTSCTS_EN;
newline.flow_control = 0;
if (memcmp(&gb_tty->line_coding, &newline, sizeof(newline))) { memcpy(&gb_tty->line_coding, &newline, sizeof(newline));
On Wed, Apr 29, 2020 at 03:00:44PM -0500, Alex Elder wrote:
On 4/29/20 2:00 PM, Arnd Bergmann wrote:
gcc-10 points out an uninitialized variable use:
Wow, nice, checking individual uninitialized fields within the structure.
The structure should really be zero-initialized anyway; it's passed as a structure in a message elsewhere. With your change, all fields in the structure are written, but in theory the structure could change and stack garbage could be sent over the wire.
What do you think of doing this instead? Or in addition?
struct gb_tty_line_coding newline = { };
(Presumably that would also silence the warning.)
I endorse of your change, either way.
Looks like Greg ended up applying an identical version of this patch that was submitted this week instead.
Taking a closer look at this code I noticed we have two versions of this line-coding struct which are supposed by be identical, but which could get out of sync (and have once already it turns out).
Johan