[FFmpeg-devel] [PATCH 3/3] avformat/img2dec: assert no pipe on ts_from_file

[Michael Niedermayer]

On Wed, May 08, 2024 at 12:16:44PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Help coverity with CID500302 Uninitialized scalar variable
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavformat/img2dec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> > index a40675d4341..6220003fb4d 100644
> > --- a/libavformat/img2dec.c
> > +++ b/libavformat/img2dec.c
> > @@ -25,6 +25,7 @@
> >  #define _DEFAULT_SOURCE
> >  #define _BSD_SOURCE
> >  #include <sys/stat.h>
> > +#include "libavutil/avassert.h"
> >  #include "libavutil/avstring.h"
> >  #include "libavutil/log.h"
> >  #include "libavutil/mem.h"
> > @@ -504,6 +505,7 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket *pkt)
> >      pkt->flags       |= AV_PKT_FLAG_KEY;
> >      if (s->ts_from_file) {
> >          struct stat img_stat;
> > +        av_assert0(!s->is_pipe);
> >          if (stat(filename, &img_stat)) {
> >              res = AVERROR(EIO);
> >              goto fail;
> 
> 1. You seem to refer to 1500302

yes, fixed locally


> which I can't even select any more
> because it has been dismissed.

no, it can be selected, it works here on the 3rd try
coverity sometimes ignores selecting issues
sometimes it displayes a long list of unrelated issues
sometimes it works


> 2. Why are you using av_assert0 to help coverity?

I dont understand the question


> 3. In cases like these it would be important to actually note why this
> assert can't be triggered. With a comment like "The ts_from_file option
> is not supported by piped input demuxers".

its kind of clear from the assert but i added the comment,


> 4. I just sent an alternative for this:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327035.html. I don't
> know which one should be preferred or whether the code should be left as-is.

I think if somehow ts_from_file and is_pipe are set. Thats a bug in the code
its only the combination of flags and AVOptions that prevents this.
I think an assert like in my patch makes the code more robust compared to
simply not crashing on an inconsistant state

thx

[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240508/30221da4/attachment.sig>