[FFmpeg-devel] [PATCH] avformat/tests/imf: Test ff_imf_parse_cpl_from_xml_dom cleanup on error

[Pierre-Anthony Lemieux]

On Fri, Aug 26, 2022 at 1:22 AM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> Pierre-Anthony Lemieux:
> > On Thu, Aug 25, 2022 at 1:58 PM Andreas Rheinhardt
> > <andreas.rheinhardt at outlook.com> wrote:
> >>
> >> Improves the test; also should fix Coverity issue #1512408.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> ---
> >>  libavformat/tests/imf.c | 10 ++++++----
> >>  1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavformat/tests/imf.c b/libavformat/tests/imf.c
> >> index e65629ccbc..a7b2ed3b4d 100644
> >> --- a/libavformat/tests/imf.c
> >> +++ b/libavformat/tests/imf.c
> >> @@ -338,10 +338,9 @@ static int test_cpl_parsing(void)
> >>      return 0;
> >>  }
> >>
> >> -static int test_bad_cpl_parsing(void)
> >> +static int test_bad_cpl_parsing(FFIMFCPL **cpl)
> >>  {
> >>      xmlDocPtr doc;
> >> -    FFIMFCPL *cpl;
> >>      int ret;
> >>
> >>      doc = xmlReadMemory(cpl_bad_doc, strlen(cpl_bad_doc), NULL, NULL, 0);
> >> @@ -350,7 +349,7 @@ static int test_bad_cpl_parsing(void)
> >>          return 1;
> >>      }
> >>
> >> -    ret = ff_imf_parse_cpl_from_xml_dom(doc, &cpl);
> >> +    ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl);
> >>      xmlFreeDoc(doc);
> >>      if (ret) {
> >>          printf("CPL parsing failed.\n");
> >> @@ -506,6 +505,7 @@ fail:
> >>
> >>  int main(int argc, char *argv[])
> >>  {
> >> +    FFIMFCPL *cpl;
> >>      int ret = 0;
> >>
> >>      if (test_cpl_parsing() != 0)
> >> @@ -518,8 +518,10 @@ int main(int argc, char *argv[])
> >>          ret = 1;
> >>
> >>      printf("#### The following should fail ####\n");
> >> -    if (test_bad_cpl_parsing() == 0)
> >> +    if (test_bad_cpl_parsing(&cpl) == 0)
> >>          ret = 1;
> >> +    else if (cpl)
> >> +        printf("Improper cleanup after failed CPL parsing\n");
> >
> > Shouldn't `ret` be set to 1 here as well?
> >
>
> Will do. (When run as part of FATE, the test would nevertheless fail,
> because the ref file does not contain the "Improper cleanup" line.)

Thanks, and yes.

>
> >>      printf("#### End failing test ####\n");
> >>
> >>      return ret;
> >> --
> >> 2.34.1