Stefan Beller <sbeller@xxxxxxxxxx> writes:
> Thanks Junio for a review of v11!
>
> I addressed the memory issue with the interdiff (in patch 1/7) as follows:
> Interdiff to v11:
>
> diff --git a/submodule.c b/submodule.c
> index 263cb2a..45d0967 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -219,6 +219,9 @@ void gitmodules_config(void)
> int parse_submodule_update_strategy(const char *value,
> struct submodule_update_strategy *dst)
> {
> + const char *com;
> +
> + free((void*)dst->command);
> dst->command = NULL;
> if (!strcmp(value, "none"))
> dst->type = SM_UPDATE_NONE;
> @@ -228,9 +231,10 @@ int parse_submodule_update_strategy(const char *value,
> dst->type = SM_UPDATE_REBASE;
> else if (!strcmp(value, "merge"))
> dst->type = SM_UPDATE_MERGE;
> - else if (skip_prefix(value, "!", &dst->command))
> + else if (skip_prefix(value, "!", &com)) {
> dst->type = SM_UPDATE_COMMAND;
> - else
> + dst->command = xstrdup(com);
> + } else
> return -1;
> return 0;
> }
Unless you count "I want to write differently from what was
suggested" is a desirable thing to do, I do not see a point in
favouring the above that uses an extra variable and skip_prefix()
over what I gave you as "how about" patch. But whatever.
- Is dst->command always initialized to a NULL (otherwise the
unconditional upfront free() makes it unsafe)?
- Is there a global free_something() or something_clear() function
that are used to release the resource held by a structure that
has submodule_update_strategy structure embedded in it? If so
dst->command needs to be freed there as well.
Thanks.
> Stefan Beller (7):
> submodule-config: keep update strategy around
> submodule-config: drop check against NULL
> fetching submodules: respect `submodule.fetchJobs` config option
> submodule update: direct error message to stderr
> git submodule update: have a dedicated helper for cloning
> submodule update: expose parallelism to the user
> clone: allow an explicit argument for parallel submodule clones
>
> Documentation/config.txt | 6 +
> Documentation/git-clone.txt | 6 +-
> Documentation/git-submodule.txt | 7 +-
> builtin/clone.c | 19 +++-
> builtin/fetch.c | 2 +-
> builtin/submodule--helper.c | 239 ++++++++++++++++++++++++++++++++++++++++
> git-submodule.sh | 54 ++++-----
> submodule-config.c | 18 ++-
> submodule-config.h | 2 +
> submodule.c | 39 ++++++-
> submodule.h | 18 +++
> t/t5526-fetch-submodules.sh | 14 +++
> t/t7400-submodule-basic.sh | 4 +-
> t/t7406-submodule-update.sh | 27 +++++
> 14 files changed, 406 insertions(+), 49 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html