[v2] c++tools: Fix memory leak

Message ID CACb0b4ktFWan9ex=pE5_xTWat9Qvwf0=bEV4uo-CgGJgMxqptQ@mail.gmail.com
State Committed
Commit c9bf4d4354b942af00193924cb59c4c6ab9cc4b5
Headers
Series [v2] c++tools: Fix memory leak |

Commit Message

Jonathan Wakely Oct. 21, 2021, 9:34 p.m. UTC
  On Thu, 21 Oct 2021 at 20:38, Jason Merrill wrote:

> On 10/21/21 09:28, Jonathan Wakely wrote:
> >   #else
> >     buffer = xmalloc (stat.st_size);
> >     if (!buffer)
> >       return -errno;
> > +  struct Deleter { void operator()(void* p) const { free(p); } };
> > +  std::unique_ptr<void, Deleter> guard;
>
> Don't you need to initialize guard from buffer?
>

Oops, yes!  Updated patch attached.
commit b280f6b5b4339586446eec99e49074e091c27ea5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 21 22:32:23 2021

    c++tools: Fix memory leak
    
    The allocated memory is not freed when returning early due to an error.
    
    c++tools/ChangeLog:
    
            * resolver.cc (module_resolver::read_tuple_file): Use unique_ptr
            to ensure memory is freed before returning.
  

Comments

Jason Merrill Oct. 26, 2021, 2:19 p.m. UTC | #1
On 10/21/21 17:34, Jonathan Wakely wrote:
> On Thu, 21 Oct 2021 at 20:38, Jason Merrill wrote:
> 
>     On 10/21/21 09:28, Jonathan Wakely wrote:
>      >   #else
>      >     buffer = xmalloc (stat.st_size);
>      >     if (!buffer)
>      >       return -errno;
>      > +  struct Deleter { void operator()(void* p) const { free(p); } };
>      > +  std::unique_ptr<void, Deleter> guard;
> 
>     Don't you need to initialize guard from buffer?
> 
> 
> Oops, yes!  Updated patch attached.

OK, thanks.

Jason
  

Patch

diff --git a/c++tools/resolver.cc b/c++tools/resolver.cc
index 421fdaa55fe..a1837b3ee10 100644
--- a/c++tools/resolver.cc
+++ b/c++tools/resolver.cc
@@ -23,6 +23,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "resolver.h"
 // C++
 #include <algorithm>
+#include <memory>
 // C
 #include <cstring>
 // OS
@@ -114,10 +115,17 @@  module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
   buffer = mmap (nullptr, stat.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
   if (buffer == MAP_FAILED)
     return -errno;
+  struct Deleter {
+    void operator()(void* p) const { munmap(p, size); }
+    size_t size;
+  };
+  std::unique_ptr<void, Deleter> guard(buffer, Deleter{(size_t)stat.st_size});
 #else
   buffer = xmalloc (stat.st_size);
   if (!buffer)
     return -errno;
+  struct Deleter { void operator()(void* p) const { free(p); } };
+  std::unique_ptr<void, Deleter> guard(buffer);
   if (read (fd, buffer, stat.st_size) != stat.st_size)
     return -errno;
 #endif
@@ -179,12 +187,6 @@  module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
 	}
     }
 
-#if MAPPED_READING
-  munmap (buffer, stat.st_size);
-#else
-  free (buffer);
-#endif
-
   return 0;
 }