On 30/7/19 17:48, Ahmad Fatoum wrote:
> Trying to output a single character via
> echo -a /dev/serial0-1
> currently results in garbage output after the newline, because console.c's
> fops_write discards the buffer length and passes the buffer to
> (struct cdev)::puts which only handles NUL-terminated strings.
>
> Fix this by amending (struct cdev)::puts with a new nbytes parameter,
> which is correctly propagated.
>
> Fixes: 1233570798 ("console: expose consoles in devfs")
> Cc: Bastian Krause <bst@xxxxxxxxxxxxxx>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> common/console.c | 12 ++++++------
> common/ratp/ratp.c | 12 +++++-------
> drivers/serial/efi-stdio.c | 5 +++--
> drivers/serial/serial_efi.c | 5 +++--
> fs/pstore/platform.c | 7 ++++---
> include/console.h | 2 +-
> 6 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/common/console.c b/common/console.c
> index 47ccf2e54de0..59218ea4e22f 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -253,17 +253,17 @@ static void console_set_stdoutpath(struct console_device *cdev)
> free(str);
> }
>
> -static int __console_puts(struct console_device *cdev, const char *s)
> +static int __console_puts(struct console_device *cdev, const char *s,
> + size_t nbytes)
> {
> - int n = 0;
> + int n;
>
> - while (*s) {
> + for (n = 0; n < nbytes; n++) {
> if (*s == '\n') {
> cdev->putc(cdev, '\r');
> n++;
> }
> cdev->putc(cdev, *s);
> - n++;
> s++;
> }
> return n;
> @@ -298,7 +298,7 @@ static ssize_t fops_write(struct cdev* dev, const void* buf, size_t count,
> {
> struct console_device *priv = dev->priv;
>
> - priv->puts(priv, buf);
> + priv->puts(priv, buf, count);
>
> return 0;
> }
> @@ -545,7 +545,7 @@ int console_puts(unsigned int ch, const char *str)
> if (initialized == CONSOLE_INIT_FULL) {
> for_each_console(cdev) {
> if (cdev->f_active & ch) {
> - n = cdev->puts(cdev, str);
> + n = cdev->puts(cdev, str, strlen(str));
> }
> }
> return n;
> diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
> index 9aea1786d684..e84ad221672a 100644
> --- a/common/ratp/ratp.c
> +++ b/common/ratp/ratp.c
> @@ -259,19 +259,17 @@ static int ratp_console_tstc(struct console_device *cdev)
> return kfifo_len(ctx->console_recv_fifo) ? 1 : 0;
> }
>
> -static int ratp_console_puts(struct console_device *cdev, const char *s)
> +static int ratp_console_puts(struct console_device *cdev, const char *s,
> + size_t nbytes)
> {
> struct ratp_ctx *ctx = container_of(cdev, struct ratp_ctx, ratp_console);
> - int len = 0;
> -
> - len = strlen(s);
>
> if (ratp_busy(&ctx->ratp))
> - return len;
> + return nbytes;
This didn't look right before. Should this be a return 0 instead?
Sascha?
>
> - kfifo_put(ctx->console_transmit_fifo, s, len);
> + kfifo_put(ctx->console_transmit_fifo, s, nbytes);
>
> - return len;
> + return nbytes;
> }
>
> static void ratp_console_putc(struct console_device *cdev, char c)
> diff --git a/drivers/serial/efi-stdio.c b/drivers/serial/efi-stdio.c
> index 0703f727e766..2ca89fa4f861 100644
> --- a/drivers/serial/efi-stdio.c
> +++ b/drivers/serial/efi-stdio.c
> @@ -243,12 +243,13 @@ static int efi_process_key(struct efi_console_priv *priv, const char *inp)
> return 1;
> }
>
> -static int efi_console_puts(struct console_device *cdev, const char *s)
> +static int efi_console_puts(struct console_device *cdev, const char *s,
> + size_t nbytes)
> {
> struct efi_console_priv *priv = to_efi(cdev);
> int n = 0;
>
> - while (*s) {
> + while (nbytes--) {
> if (*s == 27) {
> priv->efi_console_buffer[n] = 0;
> priv->out->output_string(priv->out,
> diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c
> index f0a2b22c2bc2..667d51f622ec 100644
> --- a/drivers/serial/serial_efi.c
> +++ b/drivers/serial/serial_efi.c
> @@ -130,13 +130,14 @@ static void efi_serial_putc(struct console_device *cdev, char c)
> serial->write(serial, &buffersize, &c);
> }
>
> -static int efi_serial_puts(struct console_device *cdev, const char *s)
> +static int efi_serial_puts(struct console_device *cdev, const char *s,
> + size_t nbytes)
> {
> struct efi_serial_port *uart = to_efi_serial_port(cdev);
> struct efi_serial_io_protocol *serial = uart->serial;
> uint32_t control;
> efi_status_t efiret;
> - unsigned long buffersize = strlen(s) * sizeof(char);
> + unsigned long buffersize = nbytes;
>
> do {
> efiret = serial->getcontrol(serial, &control);
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 755363c30999..601bfca6b025 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -67,10 +67,11 @@ static void pstore_console_write(const char *s, unsigned c)
> }
> }
>
> -static int pstore_console_puts(struct console_device *cdev, const char *s)
> +static int pstore_console_puts(struct console_device *cdev, const char *s,
> + size_t nbytes)
> {
> - pstore_console_write(s, strlen(s));
> - return strlen(s);
> + pstore_console_write(s, nbytes);
> + return nbytes;
> }
>
> static void pstore_console_putc(struct console_device *cdev, char c)
> diff --git a/include/console.h b/include/console.h
> index 673921331db7..1d86a584a7f2 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -42,7 +42,7 @@ struct console_device {
>
> int (*tstc)(struct console_device *cdev);
> void (*putc)(struct console_device *cdev, char c);
> - int (*puts)(struct console_device *cdev, const char *s);
> + int (*puts)(struct console_device *cdev, const char *s, size_t nbytes);
Thoughts on renaming this to something else? Apparently non-matching function
pointers are just a warning with gcc 8.2.1 (OSELAS.Toolchain-2018.12.0).
I'd have preferred non-mainline serial drivers to completely fail to compile.
> int (*getc)(struct console_device *cdev);
> int (*setbrg)(struct console_device *cdev, int baudrate);
> void (*flush)(struct console_device *cdev);
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
