Re: [PATCH 05/22] sequencer: allow the sequencer to take custody of malloc()ed data
From: Jakub Narębski <hidden>
Date: 2016-08-29 21:59:56
W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves like a one-shot command when it reads its configuration: memory is allocated and released only when the command exits. This is kind of okay for git-cherry-pick, which *is* a one-shot command. All the work to make the sequencer its work horse was done to allow using the functionality as a library function, though, including proper clean-up after use. This patch introduces an API to pass the responsibility of releasing certain memory to the sequencer.
So how this API would be / is meant to be used? From the following patches (which I shouldn't have to read to understand this one) it looks like it is about strdup'ed strings from option parsing. Or would there be something more in the future? Would sequencer as a library function be called multiple times, or only once? I'm trying to find out how this is solved in other places of Git code, and I have stumbled upon free_util in string_list...
quoted hunk ↗ jump to hunk
Signed-off-by: Johannes Schindelin <redacted> --- sequencer.c | 13 +++++++++++++ sequencer.h | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-)diff --git a/sequencer.c b/sequencer.c index c4b223b..b5be0f9 100644 --- a/sequencer.c +++ b/sequencer.c@@ -114,9 +114,22 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, return 1; } +void *sequencer_entrust(struct replay_opts *opts, void *set_me_free_after_use) +{ + ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc); + opts->owned[opts->owned_nr++] = set_me_free_after_use; + + return set_me_free_after_use;
I was wondering what this 'set_me_free_after_use' parameter is about; wouldn't it be more readable if this parameter was called 'owned_data' or 'owned_ptr'?
+}
+
static void remove_sequencer_state(const struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
+ int i;
+
+ for (i = 0; i < opts->owned_nr; i++)
+ free(opts->owned[i]);I guess you can remove owned data in any order, regardless if you store struct or its members first...
quoted hunk ↗ jump to hunk
+ free(opts->owned); strbuf_addf(&dir, "%s", get_dir(opts)); remove_dir_recursively(&dir, 0);diff --git a/sequencer.h b/sequencer.h index c955594..20b708a 100644 --- a/sequencer.h +++ b/sequencer.h@@ -43,8 +43,14 @@ struct replay_opts { /* Only used by REPLAY_NONE */ struct rev_info *revs; + + /* malloc()ed data entrusted to the sequencer */ + void **owned; + int owned_nr, owned_alloc;
I'm not sure about naming conventions for those types of data, but wouldn't 'owned_data' be a better name? I could be wrong here...
};
-#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, 0, 0, NULL }
+#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, 0, 0, NULL, NULL, 0, 0 }Nb. it is a pity that we cannot use named initializers for structs, so called designated inits. It would make this macro more readable.
+ +void *sequencer_entrust(struct replay_opts *opts, void *set_me_free_after_use); int sequencer_pick_revisions(struct replay_opts *opts);