Staging: speakup: Fix getting port information
PATCH 1/1 Staging: speakup: Fix getting port information
5e6dc54 broke the port information in the speakup driver:SERIAL_PORT_DFNS only gets defined if asm/serial.h is included.
Along the way, make sure that we do have information for the requested serial port number (index)
drivers/staging/speakup/serialio.c +8 -0
@@ -6,6 +6,9 @@#include "spk_priv.h" #include "serialio.h" +#include <linux/serial_core.h> +#include <asm/serial.h> +#ifndef SERIAL_PORT_DFNS #define SERIAL_PORT_DFNS #endif
@@ -26,6 +29,11 @@ const struct old_serial_port *spk_serialconst struct old_serial_port *ser = rs_table + index;int err; +if (index >sizeof (rs_table) /sizeof (*rs_table)) { +pr_info ("no port info for ttyS%d\n" , index); +return NULL ; + } +/* Divisor, bytesize and parity */ quot = ser->baud_base / baud; cval = cflag & (CSIZE |CSTOPB );
Quoted 5 lines · Samuel Thibault, on Sun 03 Jan 2016 00:25:29 +0100, wrote:
5e6dc54 broke the port information in the speakup driver:
SERIAL_PORT_DFNSonly gets defined ifasm/serial.his included.Along the way, make sure that we do have information for the requested serial port number (index)
(It'd be good to get this into 4.4 and 4.3.4).
Quoted 26 lines
Signed-off-by Samuel Thibault <[email protected]>drivers/staging/speakup/serialio.c +8
-0@@ -6,6 +6,9 @@#include "spk_priv.h" #include "serialio.h" +#include <linux/serial_core.h> +#include <asm/serial.h> +#ifndef SERIAL_PORT_DFNS #define SERIAL_PORT_DFNS #endif @@ -26,6 +29,11 @@ const struct old_serial_port *spk_serialconst struct old_serial_port *ser = rs_table + index;int err; +if (index >sizeof (rs_table) /sizeof (*rs_table)) { +pr_info ("no port info for ttyS%d\n" , index); +return NULL ; + } +/* Divisor, bytesize and parity */ quot = ser->baud_base / baud; cval = cflag & (CSIZE |CSTOPB );
Signature · 6 lines
Samuel
AUTHOR
FvwmM4 is the result of a random bit mutation on a hard
disk, presumably a result of a cosmic-ray or some such
thing.
(extrait de la page de man de FvwmM4)I had a patch which also worked, but yours may be better -- I enclose it here for your information.
staging/speakup/serialio.h renamed from staging/speakup/serialio.h.old +5 -4
@@ -1,22 +1,24 @@#ifndef _SPEAKUP_SERIAL_H#define _SPEAKUP_SERIAL_H +#include <linux/serial_core.h> #include <linux/serial.h> /* for rs_table, serial constants */ #include <linux/serial_reg.h> /* for more serial constants */ #ifndef __sparc__ -#include <linux/serial.h> +#include <asm/serial.h> #endif /* * this is cut&paste from 8250.h. Get rid of the structure, the definitions * and this whole broken driver. */ +struct old_serial_port {unsigned int uart;/* unused */ unsigned int baud_base;unsigned int port;unsigned int irq; -unsigned int flags;/* unused */ +upf_t flags;/*unused*/ };/* countdown values for serial timeouts in us */
@@ -34,7 +36,6 @@#define SPK_TIMEOUT 100#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE) -#define spk_serial_tx_busy () \ - ((inb(speakup_info.port_tts + UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY) +#define spk_serial_tx_busy () ((inb(speakup_info.port_tts + UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)#endif
Quoted 37 lines · Samuel Thibault
5e6dc54 broke the port information in the speakup driver:
SERIAL_PORT_DFNSonly gets defined ifasm/serial.his included.Along the way, make sure that we do have information for the requested serial port number (index)
Signed-off-by Samuel Thibault <[email protected]>drivers/staging/speakup/serialio.c +8
-0@@ -6,6 +6,9 @@#include "spk_priv.h" #include "serialio.h" +#include <linux/serial_core.h> +#include <asm/serial.h> +#ifndef SERIAL_PORT_DFNS #define SERIAL_PORT_DFNS #endif @@ -26,6 +29,11 @@ const struct old_serial_port *spk_serialconst struct old_serial_port *ser = rs_table + index;int err; +if (index >sizeof (rs_table) /sizeof (*rs_table)) { +pr_info ("no port info for ttyS%d\n" , index); +return NULL ; + } +/* Divisor, bytesize and parity */ quot = ser->baud_base / baud; cval = cflag & (CSIZE |CSTOPB );_______________________________________________
Speakup mailing list
[email protected]
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
Signature · 6 lines
Your life is like a penny. You're going to lose it. The question is:
How do
you spend it?
John Covici
[email protected]Quoted 2 lines · [email protected], on Sat 02 Jan 2016 19:10:36 -0500, wrote:
I had a patch which also worked, but yours may be better -- I enclose it here for your information.
Well, it's not up to serialio.h to include things for serialio.c. That however makes me realize that the culprit is actually f79b0d9 (which actually doesn't make much sense since linux/serial.h is getting included a couple of lines above...).
I don't know what this "use <linux/serial.h> instead <asm/serial.h>" warning is about, but *no* header in include/ includes asm/serial.h, so there is no way to get the SERIAL_PORT_DFNS definition just by including linux/serial.h, we really need asm/serial.h, just like 8250*.c do.
So we really need serialio.c to include linux/serial_core.h then asm/serial.h, as my patch does.
Samuel
Well, OK with me, I will use yours instead because I don't know if they will backport the thing, thanks so much for doing this.
Quoted 23 lines · Samuel Thibault
Quoted 2 lines · [email protected], on Sat 02 Jan 2016 19:10:36 -0500, wrote:
I had a patch which also worked, but yours may be better -- I enclose it here for your information.
Well, it's not up to
serialio.hto include things forserialio.c. That however makes me realize that the culprit is actually f79b0d9 (which actually doesn't make much sense sincelinux/serial.his getting included a couple of lines above...).I don't know what this "use <
linux/serial.h> instead <asm/serial.h>" warning is about, but *no* header ininclude/includesasm/serial.h, so there is no way to get theSERIAL_PORT_DFNSdefinition just by includinglinux/serial.h, we really needasm/serial.h, just like 8250*.c do.So we really need
serialio.cto includelinux/serial_core.hthenasm/serial.h, as my patch does.Samuel
_______________________________________________
Speakup mailing list
[email protected]
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
Signature · 6 lines
Your life is like a penny. You're going to lose it. The question is:
How do
you spend it?
John Covici
[email protected]Quoted 8 lines · Samuel Thibault
Quoted 2 lines · [email protected], on Sat 02 Jan 2016 19:10:36 -0500, wrote:
I had a patch which also worked, but yours may be better -- I enclose it here for your information.
Well, it's not up to
serialio.hto include things forserialio.c. That however makes me realize that the culprit is actually f79b0d9 (which actually doesn't make much sense sincelinux/serial.his getting included a couple of lines above...).
CC the guilty.
Also we have a Fixes tag. Please use it.
Quoted 8 lines
I don't know what this "use <
linux/serial.h> instead <asm/serial.h>" warning is about, but *no* header ininclude/includesasm/serial.h, so there is no way to get theSERIAL_PORT_DFNSdefinition just by includinglinux/serial.h, we really needasm/serial.h, just like 8250*.c do.So we really need
serialio.cto includelinux/serial_core.hthenasm/serial.h, as my patch does.
Ah... Ok.
regards,
dan carpenter
Quoted 10 lines · Dan Carpenter
Quoted 8 lines · Samuel Thibault
Quoted 2 lines · [email protected], on Sat 02 Jan 2016 19:10:36 -0500, wrote:
I had a patch which also worked, but yours may be better -- I enclose it here for your information.
Well, it's not up to
serialio.hto include things forserialio.c. That however makes me realize that the culprit is actually f79b0d9 (which actually doesn't make much sense sincelinux/serial.his getting included a couple of lines above...).
Btw, the patch title of that patch f79b0d9c223a ('staging: speakup:
Fixed warning <linux/serial.h> instead of <asm/serial.h>') describes exactly everything about the patch. So including the title really does help save time for everyone.
regards,
dan carpenter
Mmm, sorry. I don't submit patches often enough, so that each time I do it, there are new things to know about it :)
Thanks for the comments,
Samuel
Quoted 1 line · Samuel Thibault
5e6dc54 broke the port information in the speakup driver:
There is a correct format for this.
Patch 5e6dc548e453 ('drivers: staging: speakup: serialio: only use platform specific SERIAL_PORT_DFNS.') broke the port information ...
If you specify fewer than 12 numbers from the git hash it might not be unique next year. If you leave out the patch title then no one knows what you are talking about because we are not robots and we are better at remembering text instead if hex numbers. Also CC the guilty party instead of discussing them behind their backs.
Quoted 1 line
SERIAL_PORT_DFNSonly gets defined ifasm/serial.his included.
No, that's not true. There is a #define SERIAL_PORT_DFN at the start of the file. I am confused.
Quoted 9 lines
Along the way, make sure that we do have information for the requested serial port number (index)
Signed-off-by Samuel Thibault <[email protected]>
drivers/staging/speakup/serialio.c line 10
Quoted 5 lines
drivers/staging/speakup/serialio.c +2
-0@@ -6,6 +6,9 @@#include "spk_priv.h" #include "serialio.h" +#include <linux/serial_core.h> +#include <asm/serial.h>
This should be: <linux/serial.h> probably.
drivers/staging/speakup/serialio.c const struct old_serial_port *spk_serial · line 32
Quoted 9 lines
drivers/staging/speakup/serialio.c +2
-0+#ifndef SERIAL_PORT_DFNS #define SERIAL_PORT_DFNS #endif @@ -26,6 +29,11 @@ const struct old_serial_port *spk_serialconst struct old_serial_port *ser = rs_table + index;int err; +if (index >sizeof (rs_table) /sizeof (*rs_table)) {
This has an off-by-one bug > vs >=. Also use the ARRAY_SIZE() macro.
if (index >= ARRAY_SIZE(rs_table)) {Could you move the use of index below the check? Current static analysis tools are deficient and prefer "check first and then use" order.
regards,
dan carpenter