Skip to content
This repository was archived by the owner on Feb 12, 2025. It is now read-only.
This repository was archived by the owner on Feb 12, 2025. It is now read-only.

cstr_array_append might cause a memory leak #49

Description

@SalBakraa

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) {

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions