| Message ID | AM7PR03MB6660F858DB3C2C7EBA5AA9468FE49@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] tools/graph2dot: Don't use sizeof(AVFilterGraph), check allocation | expand |
| 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 |
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
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 --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;
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(-)