[FFmpeg-devel] [FFmpeg-cvslog] cbs_mpeg2: Fix type for marker_bit reading

[Mark Thompson]

On 24/10/17 23:34, Carl Eugen Hoyos wrote:
> 2017-10-25 0:29 GMT+02:00 Mark Thompson <sw at jkqxz.net>:
>> On 24/10/17 23:14, Carl Eugen Hoyos wrote:
>>> 2017-10-25 0:09 GMT+02:00 Mark Thompson <git at videolan.org>:
>>>> ffmpeg | branch: master | Mark Thompson <sw at jkqxz.net> | 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", &one, 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