[PATCH 2/3] dix: Shrink PropertyRec on LP64

Adam Jackson ajax at redhat.com
Tue Mar 8 12:17:09 PST 2011


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 <ajax at redhat.com>
> > ---
> >  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110308/eef42a5a/attachment.pgp>


More information about the xorg-devel mailing list