[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#657215: x11proto-video-dev: sizeof(xvEncodingInfo) vs sz_xvEncodingInfo



On Wed, Jan 25, 2012 at 08:27:17 +1100, Kevin Ryde wrote:

> Package: x11proto-video-dev
> Version: 2.3.1-2
> Severity: normal
> File: /usr/include/X11/extensions/Xvproto.h
> 
> The program foo.c below with gcc 4.6.1-3 prints
> 
>     sizeof(xvEncodingInfo) 24
>     sz_xvEncodingInfo      20
> 
> where if I'm not mistaken the two are supposed to be equal.
> 
> 
> It looks like in the xvEncodingInfo the xvRational field is padded onto
> a 4-byte boundary so
> 
>     CARD32 encoding
>     CARD16 name_size
>     CARD16 width
>     CARD16 height
>     <compiler padding 16>
>     INT32  rate.num
>     INT32  rate.den
>     CARD16 pad
>     <compiler padding 16>
>     = 24 bytes
> 
> Perhaps since the rate is 2*INT32 the pad was always supposed to be
> before, as
> 
>     typedef struct {
>       XvEncodingID encoding B32;
>       CARD16 name_size B16;
>       CARD16 width B16, height B16;
>       CARD16 pad B16;
>       xvRational rate;
>     } xvEncodingInfo;   
> 
> The server circa 1.7.7 seems to spit out 20 bytes in that form.  If I'm
> not mistaken it uses sz_xvEncodingInfo rather than sizeof, so quietly
> ignores the "pad" field at the end.  Eg. reply from QueryEncodings
> 
>   0000000: 01d9 0600 0700 0000 0100 d7bf 8243 0708  .............C..
>   0000010: 0000 0000 e63f 0d01 0000 0000 e43f 0d01  .....?.......?..
>   0000020: 0000 0000 0800 fe07 fe07 0d01 0100 0000  ................
>   0000030: 0100 0000 5856 5f49 4d41 4745            ....XV_IMAGE
> 
> The xlib XvQueryEncodings() looks like it uses sz_xvEncodingInfo too,
> but I don't have anything with more than one encoding to try to provoke
> it.
> 
> 
> Presuming what the server sends is correct (or at least is much too late
> to change now :-) then perhaps it'd be worth moving the "pad" field to
> where it is actually in effect, making the struct size match what's
> sent.
> 
> Dunno if that would have to rate as a break of binary compatibility.
> Presumably yes strictly speaking if anyone exposed that struct in their
> interface, but in practice I expect no if it's only ever meant as a
> temporary to layout data on send or receive.
> 
Sounds right to me, and matches
http://cgit.freedesktop.org/xcb/proto/tree/src/xv.xml#n110

Cheers,
Julien

Attachment: signature.asc
Description: Digital signature


Reply to: