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

[Tomas Härdin]

fre 2018-01-19 klockan 19:19 +0100 skrev 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...

No I was confusing myself. Your patch looks fine. With these changes
slice_buf is twiddled into the right state *before* init_get_bits(),
which makes sense.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180121/48dbaf02/attachment.sig>