`cstr_array_append` might cause a memory leak #49

SalBakraa · 2023-04-04T02:19:19Z

If you follow the example given in #43:

Cstr_Array line = cstr_array_make(cxx, CFLAGS_EXE, NULL);
FOREACH_FILE_IN_DIR(file, ".",
{
    if (ENDS_WITH(file, ".o"))
    {
        line = cstr_array_append(line, file);
    }
});
Cmd cmd = {
    .line = line
};
cmd_run_sync(cmd);

Won't the reference to the data created by cstr_array_append be overwritten every loop iteration?

I rewrote the function to use realloc instead of calling malloc ever iteration.

diff --git a/nobuild.h b/nobuild.h
index 1f41c42..935f660 100644
--- a/nobuild.h
+++ b/nobuild.h
@@ -434,13 +434,9 @@ int closedir(DIR *dirp)
 
 Cstr_Array cstr_array_append(Cstr_Array cstrs, Cstr cstr)
 {
-    Cstr_Array result = {
-        .count = cstrs.count + 1
-    };
-    result.elems = malloc(sizeof(result.elems[0]) * result.count);
-    memcpy(result.elems, cstrs.elems, cstrs.count * sizeof(result.elems[0]));
-    result.elems[cstrs.count] = cstr;
-    return result;
+    cstrs.elems = realloc(cstrs.elems, sizeof(Cstr) * (cstrs.count + 10));
+    if (cstrs.elems == NULL) {
+        PANIC("could not allocate memory: %s", strerror(errno));
+    }
+
+    cstrs.elems[cstrs.count++] = cstr;
+    return cstrs;
 }
 
 int cstr_ends_with(Cstr cstr, Cstr postfix)

This change caused an error in cmd_run_async, because it tries to append to an invalid ptr. So I just copied the original implementation there.

diff --git a/nobuild.h b/nobuild.h
index 1f41c42..0f5d39a 100644
--- a/nobuild.h
+++ b/nobuild.h
@@ -747,7 +758,12 @@ Pid cmd_run_async(Cmd cmd, Fd *fdin, Fd *fdout)
     }
 
     if (cpid == 0) {
-        Cstr_Array args = cstr_array_append(cmd.line, NULL);
+        Cstr_Array args = {
+            .count = cmd.line.count
+        };
+        args.elems = malloc(sizeof(Cstr) * args.count+1);
+        memcpy(args.elems, cmd.line.elems, args.count * sizeof(Cstr));
+        args.elems[args.count++] = NULL;
 
         if (fdin) {
             if (dup2(*fdin, STDIN_FILENO) < 0) {

The text was updated successfully, but these errors were encountered:

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`cstr_array_append` might cause a memory leak #49

`cstr_array_append` might cause a memory leak #49

cstr_array_append might cause a memory leak #49

cstr_array_append might cause a memory leak #49

Comments

`cstr_array_append` might cause a memory leak #49

`cstr_array_append` might cause a memory leak #49