diff mbox series

[FFmpeg-devel] tools/graph2dot: Don't use sizeof(AVFilterGraph), check allocation

Message ID AM7PR03MB6660F858DB3C2C7EBA5AA9468FE49@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Headers show
Series [FFmpeg-devel] tools/graph2dot: Don't use sizeof(AVFilterGraph), check allocation
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 22, 2021, 7:05 a.m. UTC
Use avfilter_graph_alloc() instead of av_mallocz(sizeof(AVFilterGraph))
to allocate an AVFilterGraph; this also properly allocates the graph's
internal. The current code just happened to work because it did not
make any use of said internal.

Also check the allocation; this fixes Coverity #1292528.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 tools/graph2dot.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Jan Ekström July 23, 2021, 7:11 p.m. UTC | #1
On Thu, Jul 22, 2021 at 10:05 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Use avfilter_graph_alloc() instead of av_mallocz(sizeof(AVFilterGraph))
> to allocate an AVFilterGraph; this also properly allocates the graph's
> internal. The current code just happened to work because it did not
> make any use of said internal.
>
> Also check the allocation; this fixes Coverity #1292528.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  tools/graph2dot.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/graph2dot.c b/tools/graph2dot.c
> index d5c1e4e3c7..fe1eedb9f3 100644
> --- a/tools/graph2dot.c
> +++ b/tools/graph2dot.c
> @@ -113,7 +113,7 @@ int main(int argc, char **argv)
>      FILE *outfile           = NULL;
>      FILE *infile            = NULL;
>      char *graph_string      = NULL;
> -    AVFilterGraph *graph = av_mallocz(sizeof(AVFilterGraph));
> +    AVFilterGraph *graph;

I really prefer initializing all values to something known instead of
whatever used to be in that memory block before (if calling the
initializer is possible, right away - otherwise a temporary failure
value such as nullptr), but I know we differ in opinion regarding
this, so let's not go further on this :)

>      char c;
>
>      av_log_set_level(AV_LOG_DEBUG);
> @@ -189,6 +189,12 @@ int main(int argc, char **argv)
>          *p = '\0';
>      }
>
> +    graph = avfilter_graph_alloc();
> +    if (!graph) {
> +        fprintf(stderr, "Memory allocation failure\n");
> +        return 1;
> +    }
> +

LGTM.

Jan
Andreas Rheinhardt July 24, 2021, 3:40 a.m. UTC | #2
Jan Ekström:
> On Thu, Jul 22, 2021 at 10:05 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> Use avfilter_graph_alloc() instead of av_mallocz(sizeof(AVFilterGraph))
>> to allocate an AVFilterGraph; this also properly allocates the graph's
>> internal. The current code just happened to work because it did not
>> make any use of said internal.
>>
>> Also check the allocation; this fixes Coverity #1292528.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  tools/graph2dot.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/graph2dot.c b/tools/graph2dot.c
>> index d5c1e4e3c7..fe1eedb9f3 100644
>> --- a/tools/graph2dot.c
>> +++ b/tools/graph2dot.c
>> @@ -113,7 +113,7 @@ int main(int argc, char **argv)
>>      FILE *outfile           = NULL;
>>      FILE *infile            = NULL;
>>      char *graph_string      = NULL;
>> -    AVFilterGraph *graph = av_mallocz(sizeof(AVFilterGraph));
>> +    AVFilterGraph *graph;
> 
> I really prefer initializing all values to something known instead of
> whatever used to be in that memory block before (if calling the
> initializer is possible, right away - otherwise a temporary failure
> value such as nullptr), but I know we differ in opinion regarding
> this, so let's not go further on this :)
> 
I fulfilled your wish; but only because the compiler can optimize it away.

- Andreas
diff mbox series

Patch

diff --git a/tools/graph2dot.c b/tools/graph2dot.c
index d5c1e4e3c7..fe1eedb9f3 100644
--- a/tools/graph2dot.c
+++ b/tools/graph2dot.c
@@ -113,7 +113,7 @@  int main(int argc, char **argv)
     FILE *outfile           = NULL;
     FILE *infile            = NULL;
     char *graph_string      = NULL;
-    AVFilterGraph *graph = av_mallocz(sizeof(AVFilterGraph));
+    AVFilterGraph *graph;
     char c;
 
     av_log_set_level(AV_LOG_DEBUG);
@@ -189,6 +189,12 @@  int main(int argc, char **argv)
         *p = '\0';
     }
 
+    graph = avfilter_graph_alloc();
+    if (!graph) {
+        fprintf(stderr, "Memory allocation failure\n");
+        return 1;
+    }
+
     if (avfilter_graph_parse(graph, graph_string, NULL, NULL, NULL) < 0) {
         fprintf(stderr, "Failed to parse the graph description\n");
         return 1;