On 30/7/19 17:53, Ahmad Fatoum wrote: > 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? Changed to zero in v2. > >> >> - 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. Same concern for v2 > >> 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