relocate: avoid memory leaks

[Bruno Haible]

Although relocate() in practice does not produce memory leaks (in the
sense of _repeated_ loss of memory blocks), it is sufficiently often reported
by analysis tools (valgrind, coverity) that I'm now doing something against it.


2017-05-16  Bruno Haible  <address@hidden>

        relocate: Make it easier to reclaim allocated memory.
        * lib/relocatable.h (relocate2): New declaration/macro.
        * lib/relocatable.c (relocate2): New function.
        * doc/relocatable-maint.texi (Supporting Relocation): Mention the
        relocate2 function.
        * lib/localcharset.c (relocate2): Define fallback.
        (get_charset_aliases): Invoke relocate2 instead of relocate. Free the
        allocated memory.
        * lib/javaversion.c (relocate2): Define fallback.
        (javaexec_version): Invoke relocate2 instead of relocate. Free the
        allocated memory.

diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
index 696b350..50b446b 100644
--- a/doc/relocatable-maint.texi
+++ b/doc/relocatable-maint.texi
@@ -88,6 +88,9 @@ bindtextdomain (PACKAGE, relocate (LOCALEDIR));
 
 The prototype for this function is in @file{relocatable.h}.
 
+There is also a variant of this function, named @code{relocate2}, that
+makes it easy to reclaim the memory allocated by the call.
+
 @item
 The @code{set_program_name} function can also configure some
 additional libraries to relocate files that they access, by defining
diff --git a/lib/javaversion.c b/lib/javaversion.c
index 817ba0c..f2331aa 100644
--- a/lib/javaversion.c
+++ b/lib/javaversion.c
@@ -23,11 +23,13 @@
 #include <errno.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <stdlib.h>
 
 #if ENABLE_RELOCATABLE
 # include "relocatable.h"
 #else
 # define relocate(pathname) (pathname)
+# define relocate2(pathname,allocatedp) (*(allocatedp) = NULL, (pathname))
 #endif
 
 #include "javaexec.h"
@@ -106,7 +108,8 @@ char *
 javaexec_version (void)
 {
   const char *class_name = "javaversion";
-  const char *pkgdatadir = relocate (PKGDATADIR);
+  char *malloc_pkgdatadir;
+  const char *pkgdatadir = relocate2 (PKGDATADIR, &malloc_pkgdatadir);
   const char *args[1];
   struct locals locals;
 
@@ -115,5 +118,6 @@ javaexec_version (void)
   execute_java_class (class_name, &pkgdatadir, 1, true, NULL, args,
                       false, false, execute_and_read_line, &locals);
 
+  free (malloc_pkgdatadir);
   return locals.line;
 }
diff --git a/lib/localcharset.c b/lib/localcharset.c
index 3c2b1ac..6b4f5ae 100644
--- a/lib/localcharset.c
+++ b/lib/localcharset.c
@@ -75,6 +75,7 @@
 # include "relocatable.h"
 #else
 # define relocate(pathname) (pathname)
+# define relocate2(pathname,allocatedp) (*(allocatedp) = NULL, (pathname))
 #endif
 
 /* Get LIBDIR.  */
@@ -129,6 +130,7 @@ get_charset_aliases (void)
   if (cp == NULL)
     {
 #if !(defined DARWIN7 || defined VMS || defined WINDOWS_NATIVE || defined 
__CYGWIN__ || defined OS2)
+      char *malloc_dir = NULL;
       const char *dir;
       const char *base = "charset.alias";
       char *file_name;
@@ -137,7 +139,7 @@ get_charset_aliases (void)
          necessary for running the testsuite before "make install".  */
       dir = getenv ("CHARSETALIASDIR");
       if (dir == NULL || dir[0] == '\0')
-        dir = relocate (LIBDIR);
+        dir = relocate2 (LIBDIR, &malloc_dir);
 
       /* Concatenate dir and base into freshly allocated file_name.  */
       {
@@ -154,6 +156,8 @@ get_charset_aliases (void)
           }
       }
 
+      free (malloc_dir);
+
       if (file_name == NULL)
         /* Out of memory.  Treat the file as empty.  */
         cp = "";
diff --git a/lib/relocatable.c b/lib/relocatable.c
index 189aee4..0892e3a 100644
--- a/lib/relocatable.c
+++ b/lib/relocatable.c
@@ -573,4 +573,17 @@ relocate (const char *pathname)
   return pathname;
 }
 
+/* Returns the pathname, relocated according to the current installation
+   directory.
+   This function sets *ALLOCATEDP to the allocated memory, or to NULL if
+   no memory allocation occurs.  So that, after you're done with the return
+   value, to reclaim allocated memory, you can do: free (*ALLOCATEDP).  */
+const char *
+relocate2 (const char *pathname, char **allocatedp)
+{
+  const char *result = relocate (pathname);
+  *allocatedp = (result != pathname ? (char *) result : NULL);
+  return result;
+}
+
 #endif
diff --git a/lib/relocatable.h b/lib/relocatable.h
index 38d7e68..11bd9ae 100644
--- a/lib/relocatable.h
+++ b/lib/relocatable.h
@@ -52,10 +52,29 @@ extern RELOCATABLE_DLL_EXPORTED void
    string that you can free with free() after casting it to 'char *'.  */
 extern const char * relocate (const char *pathname);
 
+/* Returns the pathname, relocated according to the current installation
+   directory.
+   This function sets *ALLOCATEDP to the allocated memory, or to NULL if
+   no memory allocation occurs.  So that, after you're done with the return
+   value, to reclaim allocated memory, you can do: free (*ALLOCATEDP).  */
+extern const char * relocate2 (const char *pathname, char **allocatedp);
+
 /* Memory management: relocate() potentially allocates memory, because it has
    to construct a fresh pathname.  If this is a problem because your program
-   calls relocate() frequently, think about caching the result.  Or free the
-   return value if it was different from the argument pathname.  */
+   calls relocate() frequently or because you want to fix all potential memory
+   leaks anyway, you have three options:
+   1) Use this idiom:
+        const char *pathname = ...;
+        const char *rel_pathname = relocate (pathname);
+        ...
+        if (rel_pathname != pathname)
+          free ((char *) rel_pathname);
+   2) Use this idiom:
+        char *allocated;
+        const char *rel_pathname = relocate2 (..., &allocated);
+        ...
+        free (allocated);
+   3) Think about caching the result.  */
 
 /* Convenience function:
    Computes the current installation prefix, based on the original
@@ -70,6 +89,7 @@ extern char * compute_curr_prefix (const char 
*orig_installprefix,
 
 /* By default, we use the hardwired pathnames.  */
 #define relocate(pathname) (pathname)
+#define relocate2(pathname,allocatedp) (*(allocatedp) = NULL, (pathname))
 
 #endif