Re: [FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading
2017-10-25 10:06 GMT+02:00 Hendrik Leppkes: > As if static analyzers always care what you can and cannot do. :p It was actually asan or ubsan iirc. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading
On Wed, Oct 25, 2017 at 12:42 AM, Mark Thompsonwrote: > On 24/10/17 23:34, Carl Eugen Hoyos wrote: >> 2017-10-25 0:29 GMT+02:00 Mark Thompson : >>> On 24/10/17 23:14, Carl Eugen Hoyos wrote: 2017-10-25 0:09 GMT+02:00 Mark Thompson : > ffmpeg | branch: master | Mark Thompson | Tue Oct 24 > 22:56:48 2017 +0100| [5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22] | > committer: Mark Thompson > > cbs_mpeg2: Fix type for marker_bit reading > >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22 > --- > > libavcodec/cbs_mpeg2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c > index d137762227..0cac29733e 100644 > --- a/libavcodec/cbs_mpeg2.c > +++ b/libavcodec/cbs_mpeg2.c > @@ -54,7 +54,7 @@ > xui(width, name, current->name) > > #define marker_bit() do { \ > -av_unused int one = 1; \ > +av_unused uint32_t one; \ > CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", , 1, > 1)); \ The commit message doesn't match the change / is this defined behaviour? >>> >>> It's only written and never read, so the initialisation isn't doing >>> anything. >> >> I asked because I believe it was reported once that a static analyzer >> protested >> on passing uninitialized stuff - that was never going to be used - as >> a parameter. >> (But that may have been an uninitialized variable that was never used, >> not a pointer.) > > I don't think pointers in general can be complained about in C without > knowledge of the target function - consider that there are destination > pointers all over the standard library which need not be initialised, like > memcpy() or scanf(). > As if static analyzers always care what you can and cannot do. :p - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading
On 24/10/17 23:34, Carl Eugen Hoyos wrote: > 2017-10-25 0:29 GMT+02:00 Mark Thompson: >> On 24/10/17 23:14, Carl Eugen Hoyos wrote: >>> 2017-10-25 0:09 GMT+02:00 Mark Thompson : ffmpeg | branch: master | Mark Thompson | Tue Oct 24 22:56:48 2017 +0100| [5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22] | committer: Mark Thompson cbs_mpeg2: Fix type for marker_bit reading > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22 --- libavcodec/cbs_mpeg2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index d137762227..0cac29733e 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -54,7 +54,7 @@ xui(width, name, current->name) #define marker_bit() do { \ -av_unused int one = 1; \ +av_unused uint32_t one; \ CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", , 1, 1)); \ >>> >>> The commit message doesn't match the change / is this defined behaviour? >> >> It's only written and never read, so the initialisation isn't doing anything. > > I asked because I believe it was reported once that a static analyzer > protested > on passing uninitialized stuff - that was never going to be used - as > a parameter. > (But that may have been an uninitialized variable that was never used, > not a pointer.) I don't think pointers in general can be complained about in C without knowledge of the target function - consider that there are destination pointers all over the standard library which need not be initialised, like memcpy() or scanf(). If a static analyser looks inside the called function it will find: """ int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc, int width, const char *name, uint32_t *write_to, uint32_t range_min, uint32_t range_max) { ... stuff not involving write_to ... *write_to = value; return 0; } """ which is I think reasonably clear-cut :) Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading
2017-10-25 0:29 GMT+02:00 Mark Thompson: > On 24/10/17 23:14, Carl Eugen Hoyos wrote: >> 2017-10-25 0:09 GMT+02:00 Mark Thompson : >>> ffmpeg | branch: master | Mark Thompson | Tue Oct 24 >>> 22:56:48 2017 +0100| [5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22] | >>> committer: Mark Thompson >>> >>> cbs_mpeg2: Fix type for marker_bit reading >>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22 >>> --- >>> >>> libavcodec/cbs_mpeg2.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c >>> index d137762227..0cac29733e 100644 >>> --- a/libavcodec/cbs_mpeg2.c >>> +++ b/libavcodec/cbs_mpeg2.c >>> @@ -54,7 +54,7 @@ >>> xui(width, name, current->name) >>> >>> #define marker_bit() do { \ >>> -av_unused int one = 1; \ >>> +av_unused uint32_t one; \ >>> CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", , 1, 1)); >>> \ >> >> The commit message doesn't match the change / is this defined behaviour? > > It's only written and never read, so the initialisation isn't doing anything. I asked because I believe it was reported once that a static analyzer protested on passing uninitialized stuff - that was never going to be used - as a parameter. (But that may have been an uninitialized variable that was never used, not a pointer.) Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading
On 24/10/17 23:14, Carl Eugen Hoyos wrote: > 2017-10-25 0:09 GMT+02:00 Mark Thompson: >> ffmpeg | branch: master | Mark Thompson | Tue Oct 24 >> 22:56:48 2017 +0100| [5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22] | committer: >> Mark Thompson >> >> cbs_mpeg2: Fix type for marker_bit reading >> >>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=5b2c71bb94d7cab23ee81b5c29388f5fadbcaf22 >> --- >> >> libavcodec/cbs_mpeg2.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c >> index d137762227..0cac29733e 100644 >> --- a/libavcodec/cbs_mpeg2.c >> +++ b/libavcodec/cbs_mpeg2.c >> @@ -54,7 +54,7 @@ >> xui(width, name, current->name) >> >> #define marker_bit() do { \ >> -av_unused int one = 1; \ >> +av_unused uint32_t one; \ >> CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", , 1, 1)); \ > > The commit message doesn't match the change / is this defined behaviour? It's only written and never read, so the initialisation isn't doing anything. (Hence also the av_unused.) The type change is fixing a warning on DJGPP (16-bit int is not compatible with uint32_t), the other part is a trivial cleanup but should probably still have been mentioned. - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel