| Message ID | 20240508023923.28209-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/cbs_jpeg: Assert that cbs_jpeg_assemble_fragment() stays within the array | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Michael Niedermayer: > Help coverity with CID500302 Uninitialized scalar variable > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer <michael@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 which I can't even select any more because it has been dismissed. 2. Why are you using av_assert0 to help coverity? 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". 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. - Andreas
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@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
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;
Help coverity with CID500302 Uninitialized scalar variable Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/img2dec.c | 2 ++ 1 file changed, 2 insertions(+)