[FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan complain

[Carl Eugen Hoyos]

2018-06-13 12:05 GMT+02:00, Li, Zhong <zhong.li at intel.com>:
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Carl Eugen Hoyos
>> Sent: Wednesday, June 13, 2018 5:42 PM
>> To: FFmpeg development discussions and patches
>> <ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan
>> complain
>>
>> 2018-05-24 11:39 GMT+02:00, Li, Zhong <zhong.li at intel.com>:
>> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
>> Behalf
>> >> Of Carl Eugen Hoyos
>> >> Sent: Thursday, May 24, 2018 5:33 PM
>> >> To: FFmpeg development discussions and patches
>> >> <ffmpeg-devel at ffmpeg.org>
>> >> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan
>> >> complain
>> >>
>> >> 2018-05-24 10:35 GMT+02:00, Li, Zhong <zhong.li at intel.com>:
>> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
>> >> Behalf
>> >> >> Of Carl Eugen Hoyos
>> >> >> Sent: Wednesday, May 23, 2018 8:32 PM
>> >> >> To: FFmpeg development discussions and patches
>> >> >> <ffmpeg-devel at ffmpeg.org>
>> >> >> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code
>> >> >> scan complain
>> >> >>
>> >> >> 2018-05-23 12:46 GMT+02:00, Zhong Li <zhong.li at intel.com>:
>> >> >> > Suppress the complain "variables 'type' is used but maybe
>> >> >> > uninitialized".
>> >> >> > ---
>> >> >> >  libavcodec/qsv.c | 5 ++++-
>> >> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >> >
>> >> >> > diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c index
>> >> >> > 45e1c25..3ff4f2c 100644
>> >> >> > --- a/libavcodec/qsv.c
>> >> >> > +++ b/libavcodec/qsv.c
>> >> >> > @@ -31,6 +31,7 @@
>> >> >> >  #include "libavutil/hwcontext.h"
>> >> >> >  #include "libavutil/hwcontext_qsv.h"
>> >> >> >  #include "libavutil/imgutils.h"
>> >> >> > +#include "libavutil/avassert.h"
>> >> >> >
>> >> >> >  #include "avcodec.h"
>> >> >> >  #include "qsv_internal.h"
>> >> >> > @@ -197,7 +198,7 @@ int
>> ff_qsv_find_surface_idx(QSVFramesContext
>> >> >> *ctx,
>> >> >> > QSVFrame *frame)
>> >> >> >
>> >> >> >  enum AVPictureType ff_qsv_map_pictype(int mfx_pic_type)  {
>> >> >> > -    enum AVPictureType type;
>> >> >> > +    enum AVPictureType type = AV_PICTURE_TYPE_NONE;
>> >> >> >      switch (mfx_pic_type & 0x7) {
>> >> >> >      case MFX_FRAMETYPE_I:
>> >> >> >          if (mfx_pic_type & MFX_FRAMETYPE_S) @@ -214,6
>> +215,8
>> >> >> @@ enum
>> >> >> > AVPictureType ff_qsv_map_pictype(int mfx_pic_type)
>> >> >> >          else
>> >> >> >              type = AV_PICTURE_TYPE_P;
>> >> >> >          break;
>> >> >> > +    default:
>> >> >> > +        av_assert0(0);
>> >> >>
>> >> >> I didn't test but I would have expected that exactly one of these
>> >> >> changes is sufficient to silence the warning, no?
>> >> >
>> >> > Thanks for review. It is not a compile warning and just found by
>> >> > Coverity Scan, I've double-confirmed this patch is useful to
>> >> > suppress the code scan complain.
>> >>
>> >> Of course, I understand.
>> >>
>> >> My question was if one of the two changes (ie either the variable
>> >> initialization or the assert) isn't enough to suppress the code scan
>> >> complain.
>> >
>> > I've confirmed that, running scan again. The complain is disappeared
>> > now.
>>
>> Then why did you push both changes instead of only one of them?
>
>  This one has been reviewed by you

My two questions were:
Isn't it enough to only change the variable initialization (and change
nothing in the switch) to silence the complain?
Isn't it enough to only add the default case to the switch (and change
nothing in the variable initialization) to silence the complain?

You committed changing both the variable initialization and added a
default case, one should have been enough.

Carl Eugen