[patch 1/4] wordexp: dont leak file streams

[Corinna Vinschen]

Hi Peter,

On Sep 14 01:04, Peter Rosin wrote:
> 2012-09-13  Peter Rosin  <peda@lysator.liu.se>
> 
> 	* libc/posix/wordexp.c (wordexp): Don't leak file streams.
> 
> Index: newlib/libc/posix/wordexp.c
> ===================================================================
> --- newlib.orig/libc/posix/wordexp.c
> +++ newlib/libc/posix/wordexp.c
> @@ -29,8 +29,8 @@
>  int
>  wordexp(const char *words, wordexp_t *pwordexp, int flags)
>  {
> -  FILE *f;
> -  FILE *f_err;
> +  FILE *f = NULL;
> +  FILE *f_err = NULL;
>    char tmp[MAXLINELEN];
>    int i = 0;
>    int offs = 0;
> @@ -143,8 +143,14 @@ wordexp(const char *words, wordexp_t *pw
>        pwordexp->we_wordv[pwordexp->we_wordc + offs + i] = NULL;
>        pwordexp->we_wordc += num_words;
>  
> -      close(fd[0]);
> -      close(fd_err[0]);
> +      if (f)
> +        fclose(f);
> +      else
> +        close(fd[0]);
> +      if (f_err)
> +        fclose(f_err);
> +      else
> +        close(fd_err[0]);

Thanks for the patch.  Is the `if (f)' and `if (f_err)' really
necessary, though?  The code path which ultimately leads to these close
calls always calls `f_err = fdopen(...)' and `f = fdopen(...)'.  AFAICS
you can always simply call fclose...

Sorry for being a bit late due to sickness and vacation, but here's a
more generic question:  Shall we really stick to depending on an
undocumented, more or less deprecated feature of bash?

Wouldn't it make more sense to kill the current wordexp implementation
and pull in code derived from one of the BSDs?  I had a quick look into
the FreeBSD implementation and it seems it wouldn't be a lot of work to
adapt it, if somebody feels up to the task.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat