editorconfig_parse() leaks memory when no .editorconfig files are found or there are no values

[siegel]

To reproduce, open a file in a libeditorconfig client from a directory whose ancestry contains no .editorconfig files. A leaks report (output of leaks <pid> on macOS will show something like this:

[irrelevant portions of stack omitted for clarity]
2   com.barebones.bbedit                  0x101cf146f editorconfig_parse + 559  editorconfig.c:507
1   libsystem_malloc.dylib             0x7fff6855dcf5 malloc + 21
0   libsystem_malloc.dylib             0x7fff6855dd9e malloc_zone_malloc + 140 
====
    99 (4.19K) << TOTAL >>
      1 (96 bytes) ROOT LEAK: 0x60000677dc20 [96]  length: 92  "/Users/siegel/Library/Containers/com.barebones.bbedit/Data/Library/Logs/BBEdit/.edi..."
      1 (96 bytes) ROOT LEAK: 0x60000677dce0 [96]  length: 85  "/Users/siegel/Library/Containers/com.barebones.bbedit/Data/Library/Logs/.editorconf..."
      1 (96 bytes) ROOT LEAK: 0x6000067822e0 [96]  length: 80  "/Users/siegel/Library/Containers/com.barebones.bbedit/Data/Library/.editorconfig"
      1 (96 bytes) ROOT LEAK: 0x600006795bc0 [96]  length: 81  "/Users/siegel/svn/trunk/BBEdit/FrameworksAndLibraries/LSPKit/LSPKit/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x600002258910 [80]  length: 65  "/Users/siegel/svn/trunk/BBEdit/LanguageModules/Java/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x6000022cd540 [80]  length: 65  "/Users/siegel/svn/trunk/BBEdit/LanguageModules/Rust/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x600002327c50 [80]  length: 72  "/Users/siegel/Library/Containers/com.barebones.bbedit/Data/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x600002327ca0 [80]  length: 67  "/Users/siegel/Library/Containers/com.barebones.bbedit/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x60000232a990 [80]  length: 67  "/Users/siegel/svn/trunk/BBEdit/FrameworksAndLibraries/.editorconfig"
      1 (80 bytes) ROOT LEAK: 0x60000232ac10 [80]  length: 74  "/Users/siegel/svn/trunk/BBEdit/FrameworksAndLibraries/LSPKit/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x6000018cae00 [64]  length: 60  "/Users/siegel/svn/trunk/BBEdit/LanguageModules/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x6000019028c0 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001bfb300 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001c284c0 [64]  length: 47  has-length-byte:  "Users/siegel/svn/trunk/BBEdit/dox/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001c2ed00 [64]  length: 60  "/Users/siegel/svn/trunk/BBEdit/LanguageModules/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001c44640 [64]  length: 47  has-length-byte:  "Users/siegel/svn/trunk/BBEdit/dox/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001cea100 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001d6bb00 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001d71a00 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001d73b00 [64]  length: 51  "/Users/siegel/svn/trunk/BBEdit/Source/.editorconfig"
      1 (64 bytes) ROOT LEAK: 0x600001d98800 [64]  length: 52  "/Users/siegel/svn/branches/BBEdit/13.5/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000010ecfc0 [48]  length: 44  "/Users/siegel/svn/trunk/BBEdit/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000010ed050 [48]  length: 37  "/Users/siegel/svn/trunk/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000010ee5b0 [48]  length: 46  "/Users/siegel/Library/Containers/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000010ee700 [48]  length: 35  "/Users/siegel/Library/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000010fc780 [48]  length: 44  "/Users/siegel/svn/trunk/BBEdit/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x600001111cb0 [48]  length: 37  "/Users/siegel/svn/trunk/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x60000113bf90 [48]  length: 37  "/Users/siegel/svn/trunk/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x60000113bfc0 [48]  length: 44  "/Users/siegel/svn/trunk/BBEdit/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x60000113de30 [48]  length: 44  "/Users/siegel/svn/trunk/BBEdit/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x6000011579f0 [48]  length: 37  "/Users/siegel/svn/trunk/.editorconfig"
      1 (48 bytes) ROOT LEAK: 0x600001157ae0 [48]  length: 44  "/Users/siegel/svn/trunk/BBEdit/.editorconfig"
[many more omitted for clarity]

The origin of the leak is the "no values" case:

    if (eh->name_value_count == 0) {  /* no value is set, just return 0. */
        free(hfp.full_filename);
        free(config_files);
        return 0;

The free(config_files) frees only the array block itself, and not the strings inside of it. This antipattern also occurs in the error case for reallocing the value array:

    if (eh->name_values == NULL) {
        free(hfp.full_filename);
        free(config_files);
        return EDITORCONFIG_PARSE_MEMORY_ERROR;
    }

I refactored the code at the cleanup: label into a separate function, and called that to ensure that the string array is properly freed. Patch follows:

diff --git a/src/lib/editorconfig.c b/src/lib/editorconfig.c
index e5262ca..6af834a 100644
--- a/src/lib/editorconfig.c
+++ b/src/lib/editorconfig.c
@@ -383,6 +383,20 @@ failure_cleanup:
     return NULL;
 }
 
+/*
+ * Free the memory used by an array of strings that was created by
+ * get_filenames().
+ */
+static void free_filenames(char **filenames)
+{
+    if (filenames != NULL) {
+        for (char** filename = filenames; *filename != NULL; filename++) {
+            free(*filename);
+        }
+        free(filenames);
+    }
+}
+
 /*
  * version number comparison
  */
@@ -564,7 +578,7 @@ int editorconfig_parse(const char* full_filename, editorconfig_handle h)
 
     if (eh->name_value_count == 0) {  /* no value is set, just return 0. */
         free(hfp.full_filename);
-        free(config_files);
+        free_filenames(config_files);
         return 0;
     }
     eh->name_values = hfp.array_name_value.name_values;
@@ -573,17 +587,12 @@ int editorconfig_parse(const char* full_filename, editorconfig_handle h)
             sizeof(editorconfig_name_value) * eh->name_value_count);
     if (eh->name_values == NULL) {
         free(hfp.full_filename);
+        free_filenames(config_files);
         return EDITORCONFIG_PARSE_MEMORY_ERROR;
     }
 
  cleanup:
-
-    if (config_files != NULL) {
-        for (config_file = config_files; *config_file != NULL; config_file++) {
-            free(*config_file);
-        }
-        free(config_files);
-    }
+    free_filenames(config_files);
     free(hfp.full_filename);
     free(hfp.editorconfig_file_dir);

[xuhdev]

Thanks, I'll look into this in the next few days. Feel free to make this into a pull request and we can discuss on code there.