Hi Đoàn
On 05/11/2022 11:34, Đoàn Trần Công Danh wrote:
On 2022-11-05 09:32:44+0100, René Scharfe <l.s.r@xxxxxx> wrote:
Why is this trickery needed? Above you write that callbacks in
builtin/bisect--helper.c can't use subcommand_fn because they need
their own argument. Can we extend subcommand_fn or use a global
variable to pass that extra thing instead? The latter may be ugly, but
at least it's valid C..
Not the author, but I fully agree with you, I think instead of adding new API
for some arbitrary subcommand_fn, I would change the subcommand_fn to
type:
int (*)(int argc, const char **argv, const char *prefix, void *context)
The last argument would be an object pointer, which will be casted to
the correct type inside the callback.
Let me cherry-picking this series on top of mine to see how things
would progress.
Unfortunately the current implementation of OPT_SUBCOMMAND relies on
returning a function pointer though a void* variable which while widely
supported is undefined behavior. I wonder if an API like
typedef int (*subcommand_fn)(int, char**, char*);
typedef int (*subcommand_ctk_fn)(int, char**, char*, void*);
struct subcommand; /* opaque */
OPT_SUBCOMMAND(name, value, func);
OPT_SUMCOMMAND_CTX(name, value, func, ctx);
int call_subcommand(struct subcommand* subcommand, int argc, char**
argv, char* prefix);
which would be used as
int sub1_fn(int argc, char** argv, char* prefix, void* ctx)
{
struct cmd_ctx cmd_ctx = ctx
...
}
int sub2_fn(int argc, char** argv, char* prefix)
{
...
}
int cmd_foo(int argc, char** argv, char* prefix)
{
struct cmd_ctx ctx = ...
struct subcommand cmd = { 0 };
...
struct option opts[] = {
OPT_SUBCOMMAND_CTX("sub1", &cmd, sub_fn, &ctx);
OPT_SUBCOMMAND("sub2", &cmd, sub2_fn);
};
argc = parse_options(argc, argv, prefix, &opts, usage, flags);
return call_subcommand(&cmd, argc, argv, prefix);
}
would be an improvement. One can avoid having to mark the ctx parameter
as UNUSED() if a subcommand does not need any context by using
OPT_SUBCOMMAND() rather than OPT_SUBCOMMAND_CTX().
The implementation of call_subcommand() would look something like
struct subcommand {
void* ctx;
int has_ctx;
union {
subcommand_fn fn;
subcommand_ctx_fn ctx_fn;
};
};
int call_subcommand(struct subcommand* subcommand, int argc, char
**argv, char* prefix)
{
if (subcommand->has_ctx)
return subcommand->ctx_fn(argc, argv, prefix,
subcommand->ctx);
else
return subcommand->fn(argc, argv, prefix);
}
Best Wishes
Phillip
