Re: [PATCH] leak in rl_filename_completion_function

[Grisha Levit]

On Wed, May 31, 2023 at 6:23 PM Grisha Levit <grishale...@gmail.com> wrote:
>
> If rl_filename_rewrite_hook returns a new string for a filename (which
> I guess only happens on macOS with bash), it is in most cases not
> free-d.

> diff --git a/lib/readline/complete.c b/lib/readline/complete.c
> index bf7cc856..99556a35 100644
> --- a/lib/readline/complete.c
> +++ b/lib/readline/complete.c
> @@ -2521,6 +2521,9 @@ rl_filename_completion_function (const char
> *text, int state)
>    convfn = dentry = 0;
>    while (directory && (entry = readdir (directory)))
>      {
> +      if (convfn != dentry)
> +        xfree (convfn);
> +
>        convfn = dentry = entry->d_name;
>        convlen = dentlen = D_NAMLEN (entry);
>
> @@ -2572,6 +2575,9 @@ rl_filename_completion_function (const char
> *text, int state)
>           users_dirname = (char *)NULL;
>         }
>
> +      if (convfn != dentry)
> +        xfree (convfn);
> +
>        return (char *)NULL;
>      }
>    else

I think this one might have gotten lost among the other reports, and
looking at it again it occurs to me this might be better solved by
just not allocating a new string in the filename rewrite hook since
rl_filename_completion_function will copy the string anyway.

diff --git a/bashline.c b/bashline.c
index 0e5373ab..dcf1ee1f 100644
--- a/bashline.c
+++ b/bashline.c
@@ -3279,12 +3279,7 @@ bash_directory_expansion (char **dirname)
 static char *
 bash_filename_rewrite_hook (char *fname, int fnlen)
 {
-  char *conv;
-
-  conv = fnx_fromfs (fname, fnlen);
-  if (conv != fname)
-    conv = savestring (conv);
-  return conv;
+  return fnx_fromfs (fname, fnlen);
 }

 /* Functions to save and restore the appropriate directory hook */
diff --git a/lib/readline/complete.c b/lib/readline/complete.c
index d531f541..974bba5e 100644
--- a/lib/readline/complete.c
+++ b/lib/readline/complete.c
@@ -2637,9 +2637,6 @@ rl_filename_completion_function (const char
*text, int state)
       else
        temp = savestring (convfn);

-      if (convfn != dentry)
-       xfree (convfn);
-
       return (temp);
     }
 }