| Message ID | HE1PR0101MB2219DE143439E923A902B7AA8F729@HE1PR0101MB2219.eurprd01.prod.exchangelabs.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avformat/tests/imf: Test ff_imf_parse_cpl_from_xml_dom cleanup on error | expand |
On Thu, Aug 25, 2022 at 1:58 PM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Improves the test; also should fix Coverity issue #1512408. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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? > printf("#### End failing test ####\n"); > > return ret; > -- > 2.34.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux: > On Thu, Aug 25, 2022 at 1:58 PM Andreas Rheinhardt > <andreas.rheinhardt@outlook.com> wrote: >> >> Improves the test; also should fix Coverity issue #1512408. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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.) >> printf("#### End failing test ####\n"); >> >> return ret; >> -- >> 2.34.1
On Fri, Aug 26, 2022 at 1:22 AM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Pierre-Anthony Lemieux: > > On Thu, Aug 25, 2022 at 1:58 PM Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com> wrote: > >> > >> Improves the test; also should fix Coverity issue #1512408. > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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
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"); printf("#### End failing test ####\n"); return ret;
Improves the test; also should fix Coverity issue #1512408. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/tests/imf.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)