Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

From: Olivier Marin <dkr+ml.git@free.fr>
Date: Jul 25, 2008, 15:20 UTC
Message-ID: <4889EF45.1040603@free.fr>
In-Reply-To: <200807242207.02195.barra_cuda@katamail.com>

Michele Ballabio a écrit :

Show 5 quoted lines

> 
>>> +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
>> Are you sure the default value is really used here?
> 
> Yes. Perhaps I don't understand your question. Can you explain what you mean?

If I read the code correctly in parse-options.c, with OPTION_CALLBACK, the default value is not "automatically" used. You can use it in your callback if you want, but because you don't, I think it's never used.

> Would you like this better, with PARSE_OPT_NONEG?

No, I'm fine with the negated option.

Show 8 quoted lines

> Or do you prefer changing the callback like this:
> 
> +static int parse_opt_shared_cb(const struct option *opt, const char *arg,
> +                              int unset)
> +{
> +       *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg);
> +       return 0;
> +}

I think it's better but what I suggested is more something like:

static int parse_opt_shared_cb(const struct option *opt, const char *arg,
			       int unset)
{
	*(int *)(opt->value) = unset ? -1 : git_config_perm("arg", arg);
	return 0;
}

int shared = -1;

{ OPTION_CALLBACK, 0, "shared", &shared,
	"permissions", "setup as shared repository",
	PARSE_OPT_OPTARG, parse_perm_callback },

if (shared >= 0)
	shared_repository = shared;

This way we do not change shared_repository during parsing, so we do not loose the initial value.

But it seems nobody care about this kind of details, so perhaps, you can just ignore this suggestion.

Olivier.