Andreas Rheinhardt: > The option tables of the various fftools (in particular ffprobe) are > arrays of OptionDef; said type contains a union of a pointer to void and > a function pointer of type int (*)(void *, const char *, const char *) > as well as a size_t. Some entries (namely the common entry for writing a > report as well as several more of ffprobe's entries) used the pointer to > void to store a pointer to functions of type int (*)(const char *) or > type int (*)(const char *, const char *); nevertheless, when the functions > are actually called in write_option (in cmdutils.c), it is done via a > pointer of the first type. > > There are two things wrong here: > 1. Pointer to void can be converted to any pointer to incomplete or > object type and back; but they are nevertheless not completely generic > pointers: There is no provision in the C standard that guarantees their > convertibility with function pointers. C90 lacks a generic function > pointer, C99 made every function pointer a generic function pointer and > still disallows the convertibility with void *. > 2. The signature of the called function differs from the signature > of the pointed-to type. This is undefined behaviour in C99 (given that > C90 lacks a way to convert function pointers at all, it doesn't say > anything about such a situation). It only works because none of the > functions this patch is about make any use of their parameters at all. > > Therefore this commit changes the type of the relevant functions > to match the type used for the call and uses the union's function > pointer to store it. This is legal even in C90. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > There are other places in the codebase that use a pointer to void to > store a function pointer. Should they be changed or not? > > fftools/cmdutils.c | 2 +- > fftools/cmdutils.h | 4 ++-- > fftools/ffprobe.c | 26 +++++++++++++------------- > 3 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c > index 9cfbc45c2b..fdcd376b76 100644 > --- a/fftools/cmdutils.c > +++ b/fftools/cmdutils.c > @@ -1046,7 +1046,7 @@ static int init_report(const char *env) > return 0; > } > > -int opt_report(const char *opt) > +int opt_report(void *optctx, const char *opt, const char *arg) > { > return init_report(NULL); > } > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h > index 6e2e0a2acb..1917510589 100644 > --- a/fftools/cmdutils.h > +++ b/fftools/cmdutils.h > @@ -99,7 +99,7 @@ int opt_default(void *optctx, const char *opt, const char > *arg); > */ > int opt_loglevel(void *optctx, const char *opt, const char *arg); > > -int opt_report(const char *opt); > +int opt_report(void *optctx, const char *opt, const char *arg); > > int opt_max_alloc(void *optctx, const char *opt, const char *arg); > > @@ -236,7 +236,7 @@ void show_help_options(const OptionDef *options, const > char *msg, int req_flags, > { "colors", OPT_EXIT, { .func_arg = show_colors }, > "show available color names" }, \ > { "loglevel", HAS_ARG, { .func_arg = opt_loglevel }, > "set logging level", "loglevel" }, \ > { "v", HAS_ARG, { .func_arg = opt_loglevel }, > "set logging level", "loglevel" }, \ > - { "report", 0, { (void*)opt_report }, > "generate a report" }, \ > + { "report", 0, { .func_arg = opt_report }, > "generate a report" }, \ > { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, > "set maximum size of a single allocated block", "bytes" }, \ > { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, > "force specific cpu flags", "flags" }, \ > { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show > program banner", "hide_banner" }, \ > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index 5aaddb0308..2380417229 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -3471,7 +3471,7 @@ static int opt_sections(void *optctx, const char *opt, > const char *arg) > return 0; > } > > -static int opt_show_versions(const char *opt, const char *arg) > +static int opt_show_versions(void *optctx, const char *opt, const char *arg) > { > mark_section_show_entries(SECTION_ID_PROGRAM_VERSION, 1, NULL); > mark_section_show_entries(SECTION_ID_LIBRARY_VERSION, 1, NULL); > @@ -3479,7 +3479,7 @@ static int opt_show_versions(const char *opt, const > char *arg) > } > > #define DEFINE_OPT_SHOW_SECTION(section, target_section_id) \ > - static int opt_show_##section(const char *opt, const char *arg) \ > + static int opt_show_##section(void *optctx, const char *opt, const char > *arg) \ > { \ > mark_section_show_entries(SECTION_ID_##target_section_id, 1, NULL); \ > return 0; \ > @@ -3514,9 +3514,9 @@ static const OptionDef real_options[] = { > { "sections", OPT_EXIT, {.func_arg = opt_sections}, "print sections > structure and section information, and exit" }, > { "show_data", OPT_BOOL, {(void*)&do_show_data}, "show packets data" > }, > { "show_data_hash", OPT_STRING | HAS_ARG, {(void*)&show_data_hash}, > "show packets data hash" }, > - { "show_error", 0, {(void*)&opt_show_error}, "show probing error" }, > - { "show_format", 0, {(void*)&opt_show_format}, "show format/container > info" }, > - { "show_frames", 0, {(void*)&opt_show_frames}, "show frames info" }, > + { "show_error", 0, { .func_arg = &opt_show_error }, "show probing > error" }, > + { "show_format", 0, { .func_arg = &opt_show_format }, "show > format/container info" }, > + { "show_frames", 0, { .func_arg = &opt_show_frames }, "show frames > info" }, > { "show_format_entry", HAS_ARG, {.func_arg = opt_show_format_entry}, > "show a particular entry from the format/container info", "entry" }, > { "show_entries", HAS_ARG, {.func_arg = opt_show_entries}, > @@ -3524,16 +3524,16 @@ static const OptionDef real_options[] = { > #if HAVE_THREADS > { "show_log", OPT_INT|HAS_ARG, {(void*)&do_show_log}, "show log" }, > #endif > - { "show_packets", 0, {(void*)&opt_show_packets}, "show packets info" }, > - { "show_programs", 0, {(void*)&opt_show_programs}, "show programs info" > }, > - { "show_streams", 0, {(void*)&opt_show_streams}, "show streams info" }, > - { "show_chapters", 0, {(void*)&opt_show_chapters}, "show chapters info" > }, > + { "show_packets", 0, { .func_arg = &opt_show_packets }, "show packets > info" }, > + { "show_programs", 0, { .func_arg = &opt_show_programs }, "show programs > info" }, > + { "show_streams", 0, { .func_arg = &opt_show_streams }, "show streams > info" }, > + { "show_chapters", 0, { .func_arg = &opt_show_chapters }, "show chapters > info" }, > { "count_frames", OPT_BOOL, {(void*)&do_count_frames}, "count the number > of frames per stream" }, > { "count_packets", OPT_BOOL, {(void*)&do_count_packets}, "count the > number of packets per stream" }, > - { "show_program_version", 0, {(void*)&opt_show_program_version}, "show > ffprobe version" }, > - { "show_library_versions", 0, {(void*)&opt_show_library_versions}, "show > library versions" }, > - { "show_versions", 0, {(void*)&opt_show_versions}, "show program > and library versions" }, > - { "show_pixel_formats", 0, {(void*)&opt_show_pixel_formats}, "show pixel > format descriptions" }, > + { "show_program_version", 0, { .func_arg = &opt_show_program_version }, > "show ffprobe version" }, > + { "show_library_versions", 0, { .func_arg = &opt_show_library_versions > }, "show library versions" }, > + { "show_versions", 0, { .func_arg = &opt_show_versions }, "show > program and library versions" }, > + { "show_pixel_formats", 0, { .func_arg = &opt_show_pixel_formats }, > "show pixel format descriptions" }, > { "show_private_data", OPT_BOOL, {(void*)&show_private_data}, "show > private data" }, > { "private", OPT_BOOL, {(void*)&show_private_data}, "same as > show_private_data" }, > { "bitexact", OPT_BOOL, {&do_bitexact}, "force bitexact output" }, > Ping.
- Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".