Re: [PATCH 1/2] parse-options: add int value pointer to struct option

[Taylor Blau <me@xxxxxxxxxxxx>]

On Sat, Sep 09, 2023 at 11:10:36PM +0200, René Scharfe wrote:
> Add an int pointer, value_int, to struct option to provide a typed value
> pointer for the various integer options.  It allows type checks at
> compile time, which is not possible with the void pointer, value.  Its
> use is optional for now.

This is an interesting direction. I wonder about whether or not you'd
consider changing the option structure to contain a tagged union type
that represents some common cases we'd want from a parse-options
callback, something like:

    struct option {
        /* ... */
        union {
            void *value;
            int *value_int;
            /* etc ... */
        } u;
        enum option_type t;
    };

where option_type has some value corresponding to "void *", another for
"int *", and so on.

Alternatively, perhaps you are thinking that we'd use both the value
pointer and the value_int pointer to point at potentially different
values in the same callback. I don't have strong feelings about it, but
I'd just as soon encourage us to shy away from that approach, since
assigning a single callback parameter to each function seems more
organized.

> @@ -109,6 +110,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
>  	const char *s, *arg;
>  	const int unset = flags & OPT_UNSET;
>  	int err;
> +	int *value_int = opt->value_int ? opt->value_int : opt->value;
>
>  	if (unset && p->opt)
>  		return error(_("%s takes no value"), optname(opt, flags));

Reading this hunk, I wonder whether we even need a type tag (the
option_type enum above) if each callback knows a priori what type it
expects. But I think storing them together in a union makes sense to do.

Thanks,
Taylor