[PATCH v3 0/1] Memory leak fix

[David Kahurani]

The following patch fixes a number of possible memory leaks that are
caused by overwritting of the original pointer.

V2: Resent to add a signature to the patch.

v3: Fix build issues

David Kahurani (1):
  tools/winedump : Avoid potential memory leaks

 tools/winedump/msmangle.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v3 1/1] tools/winedump : Avoid potential memory leaks

[David Kahurani]

ct->expression gets overwritten with each call to strmake. Save
its address in a temporary variable inorder to free it later
on and avoid a memory leak

Signed-off-by: David Kahurani <k.kahurani(a)gmail.com>
---
 tools/winedump/msmangle.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/winedump/msmangle.c b/tools/winedump/msmangle.c
index cf26a07..a087ae8 100644
--- a/tools/winedump/msmangle.c
+++ b/tools/winedump/msmangle.c
@@ -533,7 +533,6 @@ static char *demangle_datatype (char **str, compound_type *ct,
 	  if (*iter == '6')
 	  {
 	      int sub_expressions = 0;
-              /* FIXME: there are tons of memory leaks here */
 	      /* FIXME: this is still broken in some cases and it has to be
 	       * merged with the function prototype parsing above...
 	       */
@@ -550,15 +549,25 @@ static char *demangle_datatype (char **str, compound_type *ct,
 		      if (!demangle_datatype (&iter, &sub_ct, sym))
 			  return NULL;
 		      if (sub_expressions)
+          {
+                          char *tmp = ct->expression;
                           ct->expression = strmake( "%s, %s", ct->expression, sub_ct.expression );
+                          free(tmp);
+          }
 		      else
+          {
+                          char *tmp = ct->expression;
                           ct->expression = strmake( "%s%s", ct->expression, sub_ct.expression );
+                          free(tmp);
+          }
 		      while (*iter == '@') iter++;
 		      sub_expressions++;
 		  }
 	      } else while (*iter == '@') iter++;
 	      iter++;
+	      char *tmp = ct->expression;
 	      ct->expression = strmake( "%s)", ct->expression );
+	      free(tmp);
 	  }
 	  else
 	      return NULL;
-- 
2.33.0

[PATCH v3 1/1] tools/winedump : Avoid potential memory leaks

[Zebediah Figura (she/her)]

On 10/9/21 08:00, David Kahurani wrote:
> ct->expression gets overwritten with each call to strmake. Save
> its address in a temporary variable inorder to free it later
> on and avoid a memory leak
>
> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com>
> ---
>   tools/winedump/msmangle.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)

Thanks for the patch, but we don't consider it worthwhile to free memory 
in short-lived programs.

[PATCH v3 1/1] tools/winedump : Avoid potential memory leaks

[Stefan Dösinger]

> Am 09.10.2021 um 18:28 schrieb Zebediah Figura (she/her) <zfigura(a)codeweavers.com>:
>
> Thanks for the patch, but we don't consider it worthwhile to free memory in short-lived programs.

In that case the "FIXME: there are tons of memory leaks here" comment should maybe be updated to reflect that :-)

[PATCH v3 1/1] tools/winedump : Avoid potential memory leaks

[David K.]

Code that leaks memory is generally ugly, besides the fact that the project
might not find it worthwhile to work on this particular issue as the
program is short lived.

This patch still looks worthwhile to me, now that it has already been made
:-)

On Sun, Oct 10, 2021 at 4:15 PM Stefan Dösinger <stefandoesinger(a)gmail.com>
wrote:
> > Am 09.10.2021 um 18:28 schrieb Zebediah Figura (she/her) <
> > zfigura(a)codeweavers.com>:
> >
> > Thanks for the patch, but we don't consider it worthwhile to free memory
> > in short-lived programs.
>
> In that case the "FIXME: there are tons of memory leaks here" comment
> should maybe be updated to reflect that :-)