diff mbox series

[FFmpeg-devel,46/47] fftools/ffprobe: stop calling exit_program()

Message ID 20230715104611.17902-46-anton@khirnov.net
State Accepted
Commit a81d9231b91dac75a8fa1cbb72ec17556b5c6d9f
Headers show
Series [FFmpeg-devel,01/47] fftools/ffmpeg_mux_init: handle pixel format endianness | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov July 15, 2023, 10:46 a.m. UTC 
Inline the relevant part of ffprobe_cleanup() into main() and drop the
rest.
---
 fftools/ffprobe.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Comments

Marton Balint July 20, 2023, 8:35 p.m. UTC | #1 
On Sat, 15 Jul 2023, Anton Khirnov wrote:

> Inline the relevant part of ffprobe_cleanup() into main() and drop the
> rest.
> ---
> fftools/ffprobe.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index e234c92904..a39185f6fe 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -381,17 +381,6 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
> #endif
> }
>


[...]

> @@ -4124,8 +4112,10 @@ int main(int argc, char **argv)
>
>     show_banner(argc, argv, options);
>     ret = parse_options(NULL, argc, argv, options, opt_input_file);
> -    if (ret < 0)
> -        exit_program(ret == AVERROR_EXIT ? 0 : 1);
> +    if (ret < 0) {
> +        ret = AVERROR_EXIT ? 0 : ret;

This looks unintended.

Regards,
Marton
Anton Khirnov July 21, 2023, 12:06 p.m. UTC | #2 
Quoting Marton Balint (2023-07-20 22:35:23)
> 
> 
> On Sat, 15 Jul 2023, Anton Khirnov wrote:
> 
> > Inline the relevant part of ffprobe_cleanup() into main() and drop the
> > rest.
> > ---
> > fftools/ffprobe.c | 22 ++++++++--------------
> > 1 file changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index e234c92904..a39185f6fe 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -381,17 +381,6 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
> > #endif
> > }
> >
> 
> 
> [...]
> 
> > @@ -4124,8 +4112,10 @@ int main(int argc, char **argv)
> >
> >     show_banner(argc, argv, options);
> >     ret = parse_options(NULL, argc, argv, options, opt_input_file);
> > -    if (ret < 0)
> > -        exit_program(ret == AVERROR_EXIT ? 0 : 1);
> > +    if (ret < 0) {
> > +        ret = AVERROR_EXIT ? 0 : ret;
> 
> This looks unintended.

Which part of it and why?
Anton Khirnov July 21, 2023, 12:14 p.m. UTC | #3 
Quoting Anton Khirnov (2023-07-21 14:06:37)
> Quoting Marton Balint (2023-07-20 22:35:23)
> > 
> > 
> > On Sat, 15 Jul 2023, Anton Khirnov wrote:
> > 
> > > Inline the relevant part of ffprobe_cleanup() into main() and drop the
> > > rest.
> > > ---
> > > fftools/ffprobe.c | 22 ++++++++--------------
> > > 1 file changed, 8 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > > index e234c92904..a39185f6fe 100644
> > > --- a/fftools/ffprobe.c
> > > +++ b/fftools/ffprobe.c
> > > @@ -381,17 +381,6 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
> > > #endif
> > > }
> > >
> > 
> > 
> > [...]
> > 
> > > @@ -4124,8 +4112,10 @@ int main(int argc, char **argv)
> > >
> > >     show_banner(argc, argv, options);
> > >     ret = parse_options(NULL, argc, argv, options, opt_input_file);
> > > -    if (ret < 0)
> > > -        exit_program(ret == AVERROR_EXIT ? 0 : 1);
> > > +    if (ret < 0) {
> > > +        ret = AVERROR_EXIT ? 0 : ret;
> > 
> > This looks unintended.
> 
> Which part of it and why?

Never mind, I see it now. I'll send a patch.

Thanks,
Stefano Sabatini Aug. 2, 2023, 5:32 a.m. UTC | #4 
On date Saturday 2023-07-15 12:46:10 +0200, Anton Khirnov wrote:
> Inline the relevant part of ffprobe_cleanup() into main() and drop the
> rest.
> ---
>  fftools/ffprobe.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index e234c92904..a39185f6fe 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -381,17 +381,6 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
>  #endif
>  }
>  
> -static void ffprobe_cleanup(int ret)
> -{

> -    int i;
> -    for (i = 0; i < FF_ARRAY_ELEMS(sections); i++)
> -        av_dict_free(&(sections[i].entries_to_show));
> -

Was this part discarded? This is mostly needed to avoid valgrind
warnings.

LGTM otherwise.
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index e234c92904..a39185f6fe 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -381,17 +381,6 @@  static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
 #endif
 }
 
-static void ffprobe_cleanup(int ret)
-{
-    int i;
-    for (i = 0; i < FF_ARRAY_ELEMS(sections); i++)
-        av_dict_free(&(sections[i].entries_to_show));
-
-#if HAVE_THREADS
-    pthread_mutex_destroy(&log_mutex);
-#endif
-}
-
 struct unit_value {
     union { double d; long long int i; } val;
     const char *unit;
@@ -4113,7 +4102,6 @@  int main(int argc, char **argv)
     }
 #endif
     av_log_set_flags(AV_LOG_SKIP_REPEATED);
-    register_exit(ffprobe_cleanup);
 
     options = real_options;
     parse_loglevel(argc, argv, options);
@@ -4124,8 +4112,10 @@  int main(int argc, char **argv)
 
     show_banner(argc, argv, options);
     ret = parse_options(NULL, argc, argv, options, opt_input_file);
-    if (ret < 0)
-        exit_program(ret == AVERROR_EXIT ? 0 : 1);
+    if (ret < 0) {
+        ret = AVERROR_EXIT ? 0 : ret;
+        goto end;
+    }
 
     if (do_show_log)
         av_log_set_callback(log_callback);
@@ -4249,5 +4239,9 @@  end:
 
     avformat_network_deinit();
 
+#if HAVE_THREADS
+    pthread_mutex_destroy(&log_mutex);
+#endif
+
     return ret < 0;
 }