| Message ID | 20220104061056.32737-1-pal@sandflow.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v1] avformat/imf: fix CPL parsing error handling | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | fail | Make fate failed |
On 4/1/22 16:10, pal@sandflow.com wrote: > From: Pierre-Anthony Lemieux <pal@palemieux.com> > > Signed-off-by: Pierre-Anthony Lemieux <pal@palemieux.com> > --- > libavformat/imf_cpl.c | 51 +++++++++++++++++++++++-------------------- > 1 file changed, 27 insertions(+), 24 deletions(-) Could you please resend this as two separate commits? One with the change, one with the re-indenting? It makes it easier to review (and bisect) in the future. For reference, the effective change is: diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c index 28798d3e36..366a1be9e2 100644 --- a/libavformat/imf_cpl.c +++ b/libavformat/imf_cpl.c @@ -807,7 +807,9 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) av_log(NULL, AV_LOG_ERROR, "Cannot read IMF CPL\n"); if (ret == 0) ret = AVERROR_INVALIDDATA; - } else { + goto clean_up; + } + LIBXML_TEST_VERSION filesize = buf.len; @@ -817,6 +819,7 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) AV_LOG_ERROR, "XML parsing failed when reading the IMF CPL\n"); ret = AVERROR_INVALIDDATA; + goto clean_up; } if ((ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl))) { @@ -833,8 +836,8 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) } xmlFreeDoc(doc); - } +clean_up: av_bprint_finalize(&buf, NULL); return ret; > + av_log(NULL, > + AV_LOG_INFO, > + "IMF CPL Id: " FF_IMF_UUID_FORMAT "\n", > + UID_ARG((*cpl)->id_uuid)); > + } > + > + xmlFreeDoc(doc); > + Trailing whitespace here.
On Tue, Jan 4, 2022 at 4:58 PM Zane van Iperen <zane@zanevaniperen.com> wrote: > > > > On 4/1/22 16:10, pal@sandflow.com wrote: > > From: Pierre-Anthony Lemieux <pal@palemieux.com> > > > > Signed-off-by: Pierre-Anthony Lemieux <pal@palemieux.com> > > --- > > libavformat/imf_cpl.c | 51 +++++++++++++++++++++++-------------------- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > Could you please resend this as two separate commits? One with the change, one > with the re-indenting? It makes it easier to review (and bisect) in the future. Ok. Got it. See v2 at: http://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290840.html > > For reference, the effective change is: > > diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c > index 28798d3e36..366a1be9e2 100644 > --- a/libavformat/imf_cpl.c > +++ b/libavformat/imf_cpl.c > @@ -807,7 +807,9 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) > av_log(NULL, AV_LOG_ERROR, "Cannot read IMF CPL\n"); > if (ret == 0) > ret = AVERROR_INVALIDDATA; > - } else { > + goto clean_up; > + } > + > LIBXML_TEST_VERSION > > filesize = buf.len; > @@ -817,6 +819,7 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) > AV_LOG_ERROR, > "XML parsing failed when reading the IMF CPL\n"); > ret = AVERROR_INVALIDDATA; > + goto clean_up; > } > > if ((ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl))) { > @@ -833,8 +836,8 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) > } > > xmlFreeDoc(doc); > - } > > +clean_up: > av_bprint_finalize(&buf, NULL); > > return ret; > > > > + av_log(NULL, > > + AV_LOG_INFO, > > + "IMF CPL Id: " FF_IMF_UUID_FORMAT "\n", > > + UID_ARG((*cpl)->id_uuid)); > > + } > > + > > + xmlFreeDoc(doc); > > + > > Trailing whitespace here. > >
diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c index 167244a5a2..72fc7ffec8 100644 --- a/libavformat/imf_cpl.c +++ b/libavformat/imf_cpl.c @@ -807,34 +807,37 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) av_log(NULL, AV_LOG_ERROR, "Cannot read IMF CPL\n"); if (ret == 0) ret = AVERROR_INVALIDDATA; - } else { - LIBXML_TEST_VERSION - - filesize = buf.len; - doc = xmlReadMemory(buf.str, filesize, NULL, NULL, 0); - if (!doc) { - av_log(NULL, - AV_LOG_ERROR, - "XML parsing failed when reading the IMF CPL\n"); - ret = AVERROR_INVALIDDATA; - } + goto clean_up; + } - if ((ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl))) { - av_log(NULL, AV_LOG_ERROR, "Cannot parse IMF CPL\n"); - } else { - av_log(NULL, - AV_LOG_INFO, - "IMF CPL ContentTitle: %s\n", - (*cpl)->content_title_utf8); - av_log(NULL, - AV_LOG_INFO, - "IMF CPL Id: " FF_IMF_UUID_FORMAT "\n", - UID_ARG((*cpl)->id_uuid)); - } + LIBXML_TEST_VERSION - xmlFreeDoc(doc); + filesize = buf.len; + doc = xmlReadMemory(buf.str, filesize, NULL, NULL, 0); + if (!doc) { + av_log(NULL, + AV_LOG_ERROR, + "XML parsing failed when reading the IMF CPL\n"); + ret = AVERROR_INVALIDDATA; + goto clean_up; } + if ((ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl))) { + av_log(NULL, AV_LOG_ERROR, "Cannot parse IMF CPL\n"); + } else { + av_log(NULL, + AV_LOG_INFO, + "IMF CPL ContentTitle: %s\n", + (*cpl)->content_title_utf8); + av_log(NULL, + AV_LOG_INFO, + "IMF CPL Id: " FF_IMF_UUID_FORMAT "\n", + UID_ARG((*cpl)->id_uuid)); + } + + xmlFreeDoc(doc); + +clean_up: av_bprint_finalize(&buf, NULL); return ret;