Am 05.11.22 um 14:52 schrieb Ævar Arnfjörð Bjarmason: > > I think that's an "unportable" extension covered in "J.5 Common > extensions", specifically "J.5.7 Function pointer casts": > > A pointer to an object or to void may be cast to a pointer to a > function, allowing data to be invoked as a function > > Thus, since the standard already establishes that valid "void *" and > "intptr_t" pointers can be cast'd back & forth, the J.5.7 bridges the > gap between the two saying a function pointer can be converted to > either. > > Now, I may be missing something here, but I was under the impression > that "intptr_t" wasn't special in any way here, and that any casting of > a function pointer to either it or a "void *" was what was made portable > by "J.5.7". Do you mean "possible" or "workable" instead of "portable" here? As you write above, J.5.7 is an extension, not (fully) portable. > Anyway, like ssize_t and a few other things this is extended upon and > made standard by POSIX. I.e. we're basically talking about whether this > passes: > > assert(sizeof(void (*)(void)) == sizeof(void*)) > > And per POSIX > (https://pubs.opengroup.org/onlinepubs/9699919799/functions/dlsym.html): > > Note that conversion from a void * pointer to a function pointer > as in: > > fptr = (int (*)(int))dlsym(handle, "my_function"); > > is not defined by the ISO C standard. This standard requires > this conversion to work correctly on conforming implementations. Conversion from object pointer to function pointer can still work if function pointers are wider. > So I think aside from other concerns this should be safe to use, as > real-world data backing that up we've had a intptr_t converted to a > function pointer since v2.35.0: 5cb28270a1f (pack-objects: lazily set up > "struct rev_info", don't leak, 2022-03-28). That may not have reached unusual architectures, yet. Let's replace that cast with something boring before someone gets hurt. Something like this? diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 573d0b20b7..9e6f1530c6 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4154,14 +4154,15 @@ struct po_filter_data { struct rev_info revs; }; -static struct list_objects_filter_options *po_filter_revs_init(void *value) +static int list_objects_filter_cb(const struct option *opt, + const char *arg, int unset) { - struct po_filter_data *data = value; + struct po_filter_data *data = opt->value; repo_init_revisions(the_repository, &data->revs, NULL); data->have_revs = 1; - return &data->revs.filter; + return opt_parse_list_objects_filter(&data->revs.filter, arg, unset); } int cmd_pack_objects(int argc, const char **argv, const char *prefix) @@ -4265,7 +4266,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) &write_bitmap_index, N_("write a bitmap index if possible"), WRITE_BITMAP_QUIET, PARSE_OPT_HIDDEN), - OPT_PARSE_LIST_OBJECTS_FILTER_INIT(&pfd, po_filter_revs_init), + OPT_PARSE_LIST_OBJECTS_FILTER_F(&pfd, list_objects_filter_cb), OPT_CALLBACK_F(0, "missing", NULL, N_("action"), N_("handling for missing objects"), PARSE_OPT_NONEG, option_parse_missing_action), diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 5339660238..2e560c2fdb 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -286,15 +286,9 @@ void parse_list_objects_filter( die("%s", errbuf.buf); } -int opt_parse_list_objects_filter(const struct option *opt, +int opt_parse_list_objects_filter(struct list_objects_filter_options *filter_options, const char *arg, int unset) { - struct list_objects_filter_options *filter_options = opt->value; - opt_lof_init init = (opt_lof_init)opt->defval; - - if (init) - filter_options = init(opt->value); - if (unset || !arg) list_objects_filter_set_no_filter(filter_options); else @@ -302,6 +296,12 @@ int opt_parse_list_objects_filter(const struct option *opt, return 0; } +int opt_parse_list_objects_filter_cb(const struct option *opt, + const char *arg, int unset) +{ + return opt_parse_list_objects_filter(opt->value, arg, unset); +} + const char *list_objects_filter_spec(struct list_objects_filter_options *filter) { if (!filter->filter_spec.len) diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h index 7eeadab2dd..fc6b4da06d 100644 --- a/list-objects-filter-options.h +++ b/list-objects-filter-options.h @@ -107,31 +107,26 @@ void parse_list_objects_filter( struct list_objects_filter_options *filter_options, const char *arg); +int opt_parse_list_objects_filter(struct list_objects_filter_options *filter_options, + const char *arg, int unset); + /** * The opt->value to opt_parse_list_objects_filter() is either a * "struct list_objects_filter_option *" when using * OPT_PARSE_LIST_OBJECTS_FILTER(). * - * Or, if using no "struct option" field is used by the callback, - * except the "defval" which is expected to be an "opt_lof_init" - * function, which is called with the "opt->value" and must return a - * pointer to the ""struct list_objects_filter_option *" to be used. - * - * The OPT_PARSE_LIST_OBJECTS_FILTER_INIT() can be used e.g. the - * "struct list_objects_filter_option" is embedded in a "struct - * rev_info", which the "defval" could be tasked with lazily - * initializing. See cmd_pack_objects() for an example. + * Or, OPT_PARSE_LIST_OBJECTS_FILTER_F() can be used to specify a + * custom callback function that may expect a different type. */ -int opt_parse_list_objects_filter(const struct option *opt, - const char *arg, int unset); +int opt_parse_list_objects_filter_cb(const struct option *opt, + const char *arg, int unset); typedef struct list_objects_filter_options *(*opt_lof_init)(void *); -#define OPT_PARSE_LIST_OBJECTS_FILTER_INIT(fo, init) \ +#define OPT_PARSE_LIST_OBJECTS_FILTER_F(fo, fn) \ { OPTION_CALLBACK, 0, "filter", (fo), N_("args"), \ - N_("object filtering"), 0, opt_parse_list_objects_filter, \ - (intptr_t)(init) } + N_("object filtering"), 0, (fn) } #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \ - OPT_PARSE_LIST_OBJECTS_FILTER_INIT((fo), NULL) + OPT_PARSE_LIST_OBJECTS_FILTER_F((fo), opt_parse_list_objects_filter_cb) /* * Translates abbreviated numbers in the filter's filter_spec into their
