Thread (133 messages) 133 messages, 6 authors, 2016-06-16

Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

From: Eric Sunshine <hidden>
Date: 2016-06-16 02:18:58

On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
[off-list ref] wrote:
We cannot have a 'struct lock_file' allocated on the stack, as lockfile.c
keeps a linked list of all created lock_file structures.
By talking about "the stack" here, I suppose you mean that your
initial idea was to move the global lock_file into cmd_apply() or
something?
quoted hunk ↗ jump to hunk
So let's make the 'lock_file' variable a pointer to a 'struct lock_file'
and let's alloc the struct when needed.

Signed-off-by: Christian Couder <redacted>
---
diff --git a/builtin/apply.c b/builtin/apply.c
@@ -52,6 +52,12 @@ struct apply_state {
+       /*
+        * Since lockfile.c keeps a linked list of all created
+        * lock_file structures, it isn't safe to free(lock_file).
+        */
+       struct lock_file *lock_file;
Is there ever a time when lock_file is removed from the list (such as
upon successful write of the index), in which case it would be safe to
free() this?
quoted hunk ↗ jump to hunk
@@ -4515,8 +4521,6 @@ static int write_out_results(struct apply_state *state, struct patch *list)
        return errs;
 }

-static struct lock_file lock_file;
Does the static lock_file in build_fake_ancestor() deserve the same
sort of treatment? (I haven't traced the code enough to answer this.)
 #define INACCURATE_EOF (1<<0)
 #define RECOUNT                (1<<1)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help