On 31/7/19 09:05, Bastian Krause wrote:
> On 7/30/19 5:48 PM, 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")
> 
> The is the wrong commit id, I guess you mean b4f55fcf35.

Oh, off by one (commit). Will resend.

> 
>> Cc: Bastian Krause <[email protected]>
>> Signed-off-by: Ahmad Fatoum <[email protected]>
>> ---
>>  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;
>>  
>> -    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);
>>      int  (*getc)(struct console_device *cdev);
>>      int (*setbrg)(struct console_device *cdev, int baudrate);
>>      void (*flush)(struct console_device *cdev);
>>
> 
> Looks correct.
> 
> Bastian
> 

-- 
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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to