bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: readline fix


From: Eric Blake
Subject: Re: readline fix
Date: Thu, 10 Nov 2005 15:24:14 +0000

> 
> Some testing reveal that the readline strip any number of \n or \r at
> the end:
> 
> address@hidden:~$ cat foo.c
> #include <readline/readline.h>
> 
> int main () {
>   char *foo = readline("bar: ");
>   size_t i;
> 
>   for (i = 0; i < strlen (foo); i++)
>     printf ("%02x\n", foo[i]);
> }

Your test app was not complete (and how did it even
compile without #include <stdio.h>?).  Consider:

$ cat foo.c
#include <stdio.h>
#include <readline/readline.h>

int main () {
  char *foo = readline("bar: ");
  while (foo)
    {
      size_t i;

      for (i = 0; i < strlen (foo); i++)
        printf ("%02x ", foo[i]);
      puts ("");
      free (foo);
      foo = readline("bar: ");
    }
}
$ printf 'f\0b\r\r\n1' | ./foo
bar: 66 62 
bar:
bar:
bar: 31 
bar:

So, readline grabs the entire line, strips embedded NULs, and
delimits lines with both \r and \n.  In other words, it parsed
'f\0b\r', '\r', '\n', and finally '1', before issuing the last prompt to
recognize EOF; and it stripped \0 from within the first string,
and either a single \r or \n from the returned strings.

This means that getline() is not quite adequate; you want
something more like getndelim2, except that you want
unlimited length, and you also need to be able to strip
embedded NULs while preserving data after the NUL.

> We can't use the real size, because the readline interface doesn't
> support embedded NULs.  We must strip any and all \n and \r before the
> first NUL.

getline() already guaranteed that you only have a single \n,
but because of the problem with \0 and \r, you are going to
have to come up with a different patch.

> 
> I installed the patch below.  What do you think?
> 
> --- readline.c        10 Nov 2005 15:41:49 +0100      1.3
> +++ readline.c        10 Nov 2005 15:49:25 +0100      
> @@ -48,9 +48,8 @@
>    if (getline (&out, &size, stdin) < 0)
>      return NULL;
>  
> -  if (out[strlen (out) - 1] == '\r')
> -    out[strlen (out) - 1] = '\0';
> -  if (out[strlen (out) - 1] == '\n')
> +  while (*out && (out[strlen (out) - 1] == '\r'
> +               || out[strlen (out) - 1] == '\n'))

You are still calling strlen() too many times if this is
the approach you end up using.  Call it once, then use
a local variable to track the new length as you strip stuff
off; it will reduce your work from O(n^2) to O(n) in the
worst case of a line whose contents are all \r.

--
Eric Blake






reply via email to

[Prev in Thread] Current Thread [Next in Thread]