[FFmpeg-devel] [PATCH]lavc/svq3: Do not write into const memory

[Carl Eugen Hoyos]

2018-01-19 16:54 GMT+01:00 Tomas Härdin <tjoppen at acc.umu.se>:
> On 2018-01-18 23:34, Carl Eugen Hoyos wrote:
>>
>> Hi!
>>
>> Attached patch fixes a warning, I suspect it makes the code more correct.
>>
>> Please comment, Carl Eugen
>>
>
>> --- a/libavcodec/svq3.c
>> +++ b/libavcodec/svq3.c
>> @@ -1048,12 +1048,12 @@ static int svq3_decode_slice_header(AVCodecContext
>> *avctx)
>>           }
>>           memcpy(s->slice_buf, s->gb.buffer + s->gb.index / 8,
>> slice_bytes);
>>   -        init_get_bits(&s->gb_slice, s->slice_buf, slice_bits);
>
> Unrelated change?

No, it is an intended move.

>>           if (s->watermark_key) {
>> -            uint32_t header = AV_RL32(&s->gb_slice.buffer[1]);
>> -            AV_WL32(&s->gb_slice.buffer[1], header ^ s->watermark_key);
>
> Strange that this didn't manage to break anything, or that the compiler
> let it through

A compiler's warning that gets fixed is the reason for this patch but
I apparently misunderstand your comment.

>> +            uint32_t header = AV_RL32(&s->slice_buf[1]);
>> +            AV_WL32(&s->slice_buf[1], header ^ s->watermark_key);
>
> Considering the memcpy() above, either this or the old code must be wrong.

Why do you think so?
(I do consider the old code "wrong", that is why I sent this patch.)

> My guess is the old code must have been wrong, since to fiddle
> the same bits this AV_WL32() would need to set
> &s->slice_buf[1 - s->gb.index / 8]...

I was hoping that the same bits get fiddled given that I call the
same macro / function...

Carl Eugen