On Tue, 2011-03-08 at 19:37 +0000, Daniel Stone wrote:
> On Tue, Mar 08, 2011 at 02:13:47PM -0500, Adam Jackson wrote:
> > size needn't be a long.  No change on ILP32 but, combined with the
> > previous change, 56 -> 40 bytes on LP64.
> > 
> > Signed-off-by: Adam Jackson <[email protected]>
> > ---
> >  include/propertyst.h |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/propertyst.h b/include/propertyst.h
> > index fd1148e..1edd11d 100644
> > --- a/include/propertyst.h
> > +++ b/include/propertyst.h
> > @@ -58,8 +58,8 @@ typedef struct _Property {
> >          struct _Property       *next;
> >     ATOM            propertyName;
> >     ATOM            type;       /* ignored by server */
> > -   short           format;     /* format of data for swapping - 8,16,32 */
> > -   long            size;       /* size of data in (format/8) bytes */
> > +   uint32_t        format;     /* format of data for swapping - 8,16,32 */
> > +   uint32_t        size;       /* size of data in (format/8) bytes */
> >     pointer         data;       /* private to client */
> >     PrivateRec      *devPrivates;
> >  } PropertyRec;
> 
> Being really pedantic again, can't these two be uint8_t or something?
> You've just grown format from 16 to 32 bytes. :P

size pretty much needs to be uint32_t, MAX_BIG_REQUEST_SIZE is 4M so
even if you're using format32 you're looking at (1 << 20).  Once you've
got that, you're sticking format between two uint32_t, so making it
uint8_t would just give you 24 bits of hole.

If you really wanted to assume too much about bigreq sizing and/or throw
BadAlloc on excessively large properties, you could probably smash
format into the high bits of size, which would take you from 28 to 24
bytes per on ILP32, but wouldn't win you anything on LP64 (either you'd
be making a hole so the pointers are aligned, or you'd move it to the
end of the struct and still have padding off the end due to ABI rules).

Which seems... a little dirty, but if someone really wants that memory
back, I guess that's a thing we could do.

- ajax

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to