Re: [Bug-wget] Dangling info->lfilename

[Giuseppe Scrivano]

Hello,

"Gisle Vanem" <address@hidden> writes:

> Hello Giuseppe.
>
> You patch to mswindows.c at
>  
> http://git.savannah.gnu.org/cgit/wget.git/commit/src/mswindows.c?id=8e6de1fb5ff0ca0c749da7db634a1b1e3a1215a2
>
> @@ -123,7 +123,7 @@ struct fake_fork_info
> {
> HANDLE event;
> bool logfile_changed;
> - char lfilename[MAX_PATH + 1];
> + char *lfilename;
> };
>
> Causes a dangling pointer after use. I.e.
>  info->lfilename = strdup (opt.lfilename);
>
> seems never to be freed. We should call free() *before*
> 'info' gets unmapped or revert that change.

I am not very familiar with that part of the code, so from my
understanding that is a one-time operation and I've decided to leave the
memory leak (I thought the same happened with "info" but now I am
reading about MapViewOfFile and UnmapViewOfFile) instead of introducing
an untested free that could cause a segfault.

As I have no opportunity to test it now, does this fix solve the
problem?

-----------------------------------------------------------------------------------

diff --git a/src/mswindows.c b/src/mswindows.c
index 3bdf217..1c80421 100644
--- a/src/mswindows.c
+++ b/src/mswindows.c
@@ -293,6 +293,7 @@ fake_fork (void)
   if (info->logfile_changed)
     printf (_("Output will be written to %s.\n"), quote (info->lfilename));
 
+  free(info->lfilename);
   UnmapViewOfFile (info);
 
 cleanup:

-----------------------------------------------------------------------------------


I wonder if --background justifies all this mess in the code...maybe it
is time to obsolete it and remove in future?  We shouldn't really
reinvent the shell.  Is anyone using it?

Regards,
Giuseppe