Re: [PATCH v2 5/5] fetch: Make --jobs control submodules and remotes

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Aug 14, 2019, 08:32 UTC
Message-ID: <20190814083245.GP20404@szeder.dev>
In-Reply-To: <20190812213448.2649-6-palmer@sifive.com>

On Mon, Aug 12, 2019 at 02:34:48PM -0700, Palmer Dabbelt wrote:

Show 33 quoted lines

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 67d001f3f78b..41498e9efb3b 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -114,6 +114,20 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
>  	return git_default_config(k, v, cb);
>  }
>  
> +static int parse_jobs_arg(const struct option *opt, const char *arg, int unset)
> +{
> +	int jobs;
> +
> +	jobs = atoi(arg);
> +	if (jobs < 1)
> +		die(_("There must be a positive number of jobs"));
> +
> +	max_children_for_submodules = jobs;
> +	max_children_for_fetch = jobs;
> +
> +	return 0;
> +}
> +
>  static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
>  {
>  	BUG_ON_OPT_NEG(unset);
> @@ -142,12 +156,13 @@ static struct option builtin_fetch_options[] = {
>  		    N_("fetch all tags and associated objects"), TAGS_SET),
>  	OPT_SET_INT('n', NULL, &tags,
>  		    N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
> -	OPT_INTEGER('j', "jobs", &max_children_for_submodules,
> +	{ OPTION_CALLBACK, 'j', "jobs", NULL, N_("jobs"),
> +		    N_("number of parallel tasks to run while fetching"),
> +		    PARSE_OPT_OPTARG, &parse_jobs_arg },

These changes result segmentation faults in the tests '--quiet propagates to parallel submodules' and 'fetching submodules respects parallel settings' in 't5526-fetch-submodules.sh'.

If the number of jobs is specified as '-j 2' or '--jobs 7', i.e. as an unstuck argument of the option, as opposed to '-j2' or '--jobs=7', then 'arg' in the parse_jobs_arg() callback is NULL, which then causes the segfault somewhere inside that atoi() call.