You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Andrew Black <ab...@roguewave.com> on 2006/08/19 01:04:24 UTC
[patch] Exec utility memory leak fix
Greetings all.
Below is a short patch to fix a couple minor memory leaks in the exec
utility. This leak was likely introduced with the support for complex
targets.
--Andrew Black
Log:
* runall.cpp (merge_argv): Update function documentation.
(run_target): Free argv array returned by merge_argv.
Index: runall.cpp
===================================================================
--- runall.cpp (revision 432706)
+++ runall.cpp (working copy)
@@ -73,6 +73,9 @@
argument string is '%x' (no quotes), the contents of the provided argv
array will be inserted into the return array at that point.
+ It is the responsibility of the caller to free() the returned blocks of
+ memory, which were allocated by a call to split_opt_string().
+
@todo Figure out an escaping mechanism to allow '%x' to be passed to an
executable
@@ -402,7 +405,6 @@
static void
run_target (char* target, char** argv)
{
- struct exec_attrs status;
char** childargv;
assert (0 != target);
@@ -418,12 +420,13 @@
printf ("%-25.25s ", target_name);
fflush (stdout);
- if (!check_target_ok (childargv [0]))
- return;
+ if (check_target_ok (childargv [0])) {
+ struct exec_attrs status = exec_file (childargv);
+ process_results (childargv [0], &status);
+ }
- status = exec_file (childargv);
-
- process_results (childargv [0], &status);
+ free (childargv [0]);
+ free (childargv);
}
/**
Re: [patch] Exec utility memory leak fix
Posted by Martin Sebor <se...@roguewave.com>.
FYI: I applied the patch on Linux:
http://svn.apache.org/viewvc?rev=436919&view=rev
Martin Sebor wrote:
> Andrew Black wrote:
>
>> Greetings Martin.
>>
>> I think the problem is that the Solaris patch utility has problems
>> with whitespace fuzz (specifically with changes in trailing whitespace).
>
>
> I figured it had something to do with whitespace.
>
>>
>> When I pulled your attached copy of the patch and applied it (on my
>> linux box), I recieved the following message:
>>
>> [ablack@quandry util]$ patch < leakfix.diff
>> patching file runall.cpp
>> Hunk #1 succeeded at 73 with fuzz 1.
>
>
> I wonder if the spaces might have been introduced into the text
> of the patch by one of our mailers or by ezmlm. It might be best
> to avoid pasting patches into email and instead attach them as
> files.
>
> Martin
Re: [patch] Exec utility memory leak fix
Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:
> Greetings Martin.
>
> I think the problem is that the Solaris patch utility has problems with
> whitespace fuzz (specifically with changes in trailing whitespace).
I figured it had something to do with whitespace.
>
> When I pulled your attached copy of the patch and applied it (on my
> linux box), I recieved the following message:
>
> [ablack@quandry util]$ patch < leakfix.diff
> patching file runall.cpp
> Hunk #1 succeeded at 73 with fuzz 1.
I wonder if the spaces might have been introduced into the text
of the patch by one of our mailers or by ezmlm. It might be best
to avoid pasting patches into email and instead attach them as
files.
Martin
Re: [patch] Exec utility memory leak fix
Posted by Andrew Black <ab...@roguewave.com>.
Greetings Martin.
I think the problem is that the Solaris patch utility has problems with
whitespace fuzz (specifically with changes in trailing whitespace).
When I pulled your attached copy of the patch and applied it (on my
linux box), I recieved the following message:
[ablack@quandry util]$ patch < leakfix.diff
patching file runall.cpp
Hunk #1 succeeded at 73 with fuzz 1.
Attempting to apply it on one of our Solaris machine resulted in the
same behavior you observed (along with a failed application).
When I compared the output from svn diff after applying it with the
patch you sent back, I found the following output:
[ablack@quandry util]$ svn diff | diff -u leakfix.diff -
--- leakfix.diff 2006-08-25 10:54:56.000000000 -0600
+++ - 2006-08-25 11:04:47.000000000 -0600
@@ -1,41 +1,41 @@
Index: runall.cpp
===================================================================
---- runall.cpp (revision 432706)
-+++ runall.cpp (working copy)
+--- runall.cpp (revision 436850)
++++ runall.cpp (working copy)
@@ -73,6 +73,9 @@
- argument string is '%x' (no quotes), the contents of the provided argv
+ argument string is '%x' (no quotes), the contents of the provided argv
array will be inserted into the return array at that point.
-
+
+ It is the responsibility of the caller to free() the returned
blocks of
+ memory, which were allocated by a call to split_opt_string().
+
@todo Figure out an escaping mechanism to allow '%x' to be passed
to an
executable
-
+
@@ -402,7 +405,6 @@
static void
run_target (char* target, char** argv)
{
- struct exec_attrs status;
char** childargv;
-
+
assert (0 != target);
@@ -418,12 +420,13 @@
printf ("%-25.25s ", target_name);
fflush (stdout);
-
+
- if (!check_target_ok (childargv [0]))
- return;
+ if (check_target_ok (childargv [0])) {
+ struct exec_attrs status = exec_file (childargv);
+ process_results (childargv [0], &status);
+ }
-
+
- status = exec_file (childargv);
-
- process_results (childargv [0], &status);
+ free (childargv [0]);
+ free (childargv);
}
-
+
/**
--Andrew Black
Martin Sebor wrote:
> Andrew Black wrote:
>> Greetings all.
>>
>> Below is a short patch to fix a couple minor memory leaks in the exec
>> utility. This leak was likely introduced with the support for complex
>> targets.
>
> Hmm, I'm having trouble applying this patch on Solaris 9.
> Attached is the the patch itself (copied from your post)
> and the .rej file. Do you see what the problem is?
>
> Martin
>
[...]
Re: [patch] Exec utility memory leak fix
Posted by Andrew Black <ab...@roguewave.com>.
Martin Sebor wrote:
> Andrew Black wrote:
>> Looking at the .rej file, I'm not seeing the difference between the
>> two sections indicated.
>>
>> What does it look like if you execute 'svn diff runall.cpp | diff
>> runall.patch -' after applying the patch? If there is no difference
>> between the resulting patches, it would seem to me that the solaris
>> patch utility has issues.
>
> Sorry, I have no time to play with it today. There are known
> incompatibilities between GNU patch (I assume that's what you
> used) and other implementations of the utility so maybe it's
> one of them.
>
> Martin
>
I've been generating the patches with 'svn diff'. I don't know the
details of how it generates the patches, but it is fairly likely that
subversion uses the system's diff utility (which is the GNU diff utility
on my local box) to generate the patches.
--Andrew Black
Re: [patch] Exec utility memory leak fix
Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:
> Looking at the .rej file, I'm not seeing the difference between the two
> sections indicated.
>
> What does it look like if you execute 'svn diff runall.cpp | diff
> runall.patch -' after applying the patch? If there is no difference
> between the resulting patches, it would seem to me that the solaris
> patch utility has issues.
Sorry, I have no time to play with it today. There are known
incompatibilities between GNU patch (I assume that's what you
used) and other implementations of the utility so maybe it's
one of them.
Martin
>
> --Andrew Black
>
> Martin Sebor wrote:
>
>> Andrew Black wrote:
>>
>>> Greetings all.
>>>
>>> Below is a short patch to fix a couple minor memory leaks in the exec
>>> utility. This leak was likely introduced with the support for
>>> complex targets.
>>
>>
>> Hmm, I'm having trouble applying this patch on Solaris 9.
>> Attached is the the patch itself (copied from your post)
>> and the .rej file. Do you see what the problem is?
>>
>> Martin
>>
>>>
>>> --Andrew Black
>>>
>>> Log:
>>> * runall.cpp (merge_argv): Update function documentation.
>>> (run_target): Free argv array returned by merge_argv.
>>>
>>> Index: runall.cpp
>>> ===================================================================
>>> --- runall.cpp (revision 432706)
>>> +++ runall.cpp (working copy)
>>> @@ -73,6 +73,9 @@
>>> argument string is '%x' (no quotes), the contents of the provided
>>> argv
>>> array will be inserted into the return array at that point.
>>>
>>> + It is the responsibility of the caller to free() the returned
>>> blocks of
>>> + memory, which were allocated by a call to split_opt_string().
>>> +
>>> @todo Figure out an escaping mechanism to allow '%x' to be passed
>>> to an
>>> executable
>>>
>>> @@ -402,7 +405,6 @@
>>> static void
>>> run_target (char* target, char** argv)
>>> {
>>> - struct exec_attrs status;
>>> char** childargv;
>>>
>>> assert (0 != target);
>>> @@ -418,12 +420,13 @@
>>> printf ("%-25.25s ", target_name);
>>> fflush (stdout);
>>>
>>> - if (!check_target_ok (childargv [0]))
>>> - return;
>>> + if (check_target_ok (childargv [0])) {
>>> + struct exec_attrs status = exec_file (childargv);
>>> + process_results (childargv [0], &status);
>>> + }
>>>
>>> - status = exec_file (childargv);
>>> -
>>> - process_results (childargv [0], &status);
>>> + free (childargv [0]);
>>> + free (childargv);
>>> }
>>>
>>> /**
>>>
>>
Re: [patch] Exec utility memory leak fix
Posted by Andrew Black <ab...@roguewave.com>.
Looking at the .rej file, I'm not seeing the difference between the two
sections indicated.
What does it look like if you execute 'svn diff runall.cpp | diff
runall.patch -' after applying the patch? If there is no difference
between the resulting patches, it would seem to me that the solaris
patch utility has issues.
--Andrew Black
Martin Sebor wrote:
> Andrew Black wrote:
>> Greetings all.
>>
>> Below is a short patch to fix a couple minor memory leaks in the exec
>> utility. This leak was likely introduced with the support for complex
>> targets.
>
> Hmm, I'm having trouble applying this patch on Solaris 9.
> Attached is the the patch itself (copied from your post)
> and the .rej file. Do you see what the problem is?
>
> Martin
>
>>
>> --Andrew Black
>>
>> Log:
>> * runall.cpp (merge_argv): Update function documentation.
>> (run_target): Free argv array returned by merge_argv.
>>
>> Index: runall.cpp
>> ===================================================================
>> --- runall.cpp (revision 432706)
>> +++ runall.cpp (working copy)
>> @@ -73,6 +73,9 @@
>> argument string is '%x' (no quotes), the contents of the provided
>> argv
>> array will be inserted into the return array at that point.
>>
>> + It is the responsibility of the caller to free() the returned
>> blocks of
>> + memory, which were allocated by a call to split_opt_string().
>> +
>> @todo Figure out an escaping mechanism to allow '%x' to be passed
>> to an
>> executable
>>
>> @@ -402,7 +405,6 @@
>> static void
>> run_target (char* target, char** argv)
>> {
>> - struct exec_attrs status;
>> char** childargv;
>>
>> assert (0 != target);
>> @@ -418,12 +420,13 @@
>> printf ("%-25.25s ", target_name);
>> fflush (stdout);
>>
>> - if (!check_target_ok (childargv [0]))
>> - return;
>> + if (check_target_ok (childargv [0])) {
>> + struct exec_attrs status = exec_file (childargv);
>> + process_results (childargv [0], &status);
>> + }
>>
>> - status = exec_file (childargv);
>> -
>> - process_results (childargv [0], &status);
>> + free (childargv [0]);
>> + free (childargv);
>> }
>>
>> /**
>>
>
Re: [patch] Exec utility memory leak fix
Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:
> Greetings all.
>
> Below is a short patch to fix a couple minor memory leaks in the exec
> utility. This leak was likely introduced with the support for complex
> targets.
Hmm, I'm having trouble applying this patch on Solaris 9.
Attached is the the patch itself (copied from your post)
and the .rej file. Do you see what the problem is?
Martin
>
> --Andrew Black
>
> Log:
> * runall.cpp (merge_argv): Update function documentation.
> (run_target): Free argv array returned by merge_argv.
>
> Index: runall.cpp
> ===================================================================
> --- runall.cpp (revision 432706)
> +++ runall.cpp (working copy)
> @@ -73,6 +73,9 @@
> argument string is '%x' (no quotes), the contents of the provided argv
> array will be inserted into the return array at that point.
>
> + It is the responsibility of the caller to free() the returned blocks of
> + memory, which were allocated by a call to split_opt_string().
> +
> @todo Figure out an escaping mechanism to allow '%x' to be passed to an
> executable
>
> @@ -402,7 +405,6 @@
> static void
> run_target (char* target, char** argv)
> {
> - struct exec_attrs status;
> char** childargv;
>
> assert (0 != target);
> @@ -418,12 +420,13 @@
> printf ("%-25.25s ", target_name);
> fflush (stdout);
>
> - if (!check_target_ok (childargv [0]))
> - return;
> + if (check_target_ok (childargv [0])) {
> + struct exec_attrs status = exec_file (childargv);
> + process_results (childargv [0], &status);
> + }
>
> - status = exec_file (childargv);
> -
> - process_results (childargv [0], &status);
> + free (childargv [0]);
> + free (childargv);
> }
>
> /**
>