[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] Dangling info->lfilename
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] Dangling info->lfilename |
Date: |
Tue, 17 Jun 2014 15:59:31 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
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