| Message ID | 1527072408-11829-1-git-send-email-zhong.li@intel.com |
|---|---|
| State | Accepted |
| Commit | 8a0c2901f1fa1cd4b2cf8f347840e4465d65cbae |
| Headers | show |
2018-05-23 12:46 GMT+02:00, Zhong Li <zhong.li@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? Carl Eugen
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Wednesday, May 23, 2018 8:32 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@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@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. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2018-05-24 10:35 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Wednesday, May 23, 2018 8:32 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@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@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. Carl Eugen
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Thursday, May 24, 2018 5:33 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@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@intel.com>: > >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > >> Of Carl Eugen Hoyos > >> Sent: Wednesday, May 23, 2018 8:32 PM > >> To: FFmpeg development discussions and patches > >> <ffmpeg-devel@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@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. > > Carl Eugen
2018-05-24 11:39 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Thursday, May 24, 2018 5:33 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@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@intel.com>: >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >> Behalf >> >> Of Carl Eugen Hoyos >> >> Sent: Wednesday, May 23, 2018 8:32 PM >> >> To: FFmpeg development discussions and patches >> >> <ffmpeg-devel@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@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? Carl Eugen
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Wednesday, June 13, 2018 5:42 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@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@intel.com>: > >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > >> Of Carl Eugen Hoyos > >> Sent: Thursday, May 24, 2018 5:33 PM > >> To: FFmpeg development discussions and patches > >> <ffmpeg-devel@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@intel.com>: > >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > >> Behalf > >> >> Of Carl Eugen Hoyos > >> >> Sent: Wednesday, May 23, 2018 8:32 PM > >> >> To: FFmpeg development discussions and patches > >> >> <ffmpeg-devel@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@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 and Mark, so it is the first one to be applied. The reset will be applied soon if without comment. > > Carl Eugen
2018-06-13 12:05 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >> -----Original Message----- >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Wednesday, June 13, 2018 5:42 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@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@intel.com>: >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >> Behalf >> >> Of Carl Eugen Hoyos >> >> Sent: Thursday, May 24, 2018 5:33 PM >> >> To: FFmpeg development discussions and patches >> >> <ffmpeg-devel@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@intel.com>: >> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >> >> Behalf >> >> >> Of Carl Eugen Hoyos >> >> >> Sent: Wednesday, May 23, 2018 8:32 PM >> >> >> To: FFmpeg development discussions and patches >> >> >> <ffmpeg-devel@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@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
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Wednesday, June 13, 2018 6:11 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan > complain > > 2018-06-13 12:05 GMT+02:00, Li, Zhong <zhong.li@intel.com>: > >> -----Original Message----- > >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > >> Of Carl Eugen Hoyos > >> Sent: Wednesday, June 13, 2018 5:42 PM > >> To: FFmpeg development discussions and patches > >> <ffmpeg-devel@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@intel.com>: > >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > >> Behalf > >> >> Of Carl Eugen Hoyos > >> >> Sent: Thursday, May 24, 2018 5:33 PM > >> >> To: FFmpeg development discussions and patches > >> >> <ffmpeg-devel@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@intel.com>: > >> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] > On > >> >> Behalf > >> >> >> Of Carl Eugen Hoyos > >> >> >> Sent: Wednesday, May 23, 2018 8:32 PM > >> >> >> To: FFmpeg development discussions and patches > >> >> >> <ffmpeg-devel@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@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. Ok, I misunderstood you doubted any one of these two changes can silence the complain. Yes, one is enough for the complain. (the first version of this patch is just to change the variable initialization. Mark suggested to add the assert() to check the input is valid). So maybe you would like to keep the assert() and remove the variable initialization? (I agree if just to silence the complain, but current patch seems robust supposing anyone add code to use "type" before the assert()). > > Carl Eugen
2018-06-14 5:08 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >> -----Original Message----- >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Wednesday, June 13, 2018 6:11 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@ffmpeg.org> >> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan >> complain >> >> 2018-06-13 12:05 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >> >> -----Original Message----- >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >> Behalf >> >> Of Carl Eugen Hoyos >> >> Sent: Wednesday, June 13, 2018 5:42 PM >> >> To: FFmpeg development discussions and patches >> >> <ffmpeg-devel@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@intel.com>: >> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >> >> Behalf >> >> >> Of Carl Eugen Hoyos >> >> >> Sent: Thursday, May 24, 2018 5:33 PM >> >> >> To: FFmpeg development discussions and patches >> >> >> <ffmpeg-devel@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@intel.com>: >> >> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] >> On >> >> >> Behalf >> >> >> >> Of Carl Eugen Hoyos >> >> >> >> Sent: Wednesday, May 23, 2018 8:32 PM >> >> >> >> To: FFmpeg development discussions and patches >> >> >> >> <ffmpeg-devel@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@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. > > Ok, I misunderstood you doubted any one of these two changes can silence the > complain. > Yes, one is enough for the complain. (the first version of this patch is > just to change the variable initialization. Mark suggested to add the > assert() to check the input is valid). > So maybe you would like to keep the assert() and remove the variable > initialization? That would have been my original suggestion, yes. Carl Eugen
On 14/06/18 10:50, Carl Eugen Hoyos wrote: > 2018-06-14 5:08 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >>> -----Original Message----- >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >>> Of Carl Eugen Hoyos >>> Sent: Wednesday, June 13, 2018 6:11 PM >>> To: FFmpeg development discussions and patches >>> <ffmpeg-devel@ffmpeg.org> >>> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan >>> complain >>> >>> 2018-06-13 12:05 GMT+02:00, Li, Zhong <zhong.li@intel.com>: >>>>> -----Original Message----- >>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >>> Behalf >>>>> Of Carl Eugen Hoyos >>>>> Sent: Wednesday, June 13, 2018 5:42 PM >>>>> To: FFmpeg development discussions and patches >>>>> <ffmpeg-devel@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@intel.com>: >>>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On >>>>> Behalf >>>>>>> Of Carl Eugen Hoyos >>>>>>> Sent: Thursday, May 24, 2018 5:33 PM >>>>>>> To: FFmpeg development discussions and patches >>>>>>> <ffmpeg-devel@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@intel.com>: >>>>>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] >>> On >>>>>>> Behalf >>>>>>>>> Of Carl Eugen Hoyos >>>>>>>>> Sent: Wednesday, May 23, 2018 8:32 PM >>>>>>>>> To: FFmpeg development discussions and patches >>>>>>>>> <ffmpeg-devel@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@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. >> >> Ok, I misunderstood you doubted any one of these two changes can silence the >> complain. >> Yes, one is enough for the complain. (the first version of this patch is >> just to change the variable initialization. Mark suggested to add the >> assert() to check the input is valid). > >> So maybe you would like to keep the assert() and remove the variable >> initialization? > > That would have been my original suggestion, yes. That was what I meant too. Generally you don't want an initialisation when it isn't needed, because it makes warnings less useful. To suppress the warning the assert (which the compiler knows will not return on failure) should be sufficient. - Mark
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); } return type;