[PATCH 00/19] Reworking initializion of data sent to clients
Alan Coopersmith
alan.coopersmith at oracle.com
Sun Jun 24 10:25:07 PDT 2012
This is one of those patch series that started out as noticing one little
nit, and then when you pull on the thread the whole server unravels in
your lap and you have to spend time off and on for months knitting it back
together before you're happy again. In this case, it was noticing garbage
in padding fields of a couple requests when running xscope, which led to
looking at how we initialize padding fields and finding that, as usual,
the X server was wildly inconsistent across different extensions & areas
of the code.
This tries to simplify everything and make it consistent, using the C99
designated initializers where possible to set things up. Doing that while
respecting our avoidance of the "no declarations in the middle of a block"
rule did require making new blocks in a number of places, or using the
C99 feature of casting an anonymous struct in some others. I tendend to
prefer new blocks when reasonable, to better show through variable scope
when the struct was actually valid/needed. Where possible I tried to
make the blocks naturally fit in, as part of something like an if/else
clause, but if not, there's just a block in the middle of the code now.
This should be a little more efficient, though probably not noticably so,
as it allows the compiler more freedom in loading values into structures,
instead of requiring a certain ordering, and instead of going through all
the trouble to memset 32 bytes to 0, then overwriting 24 of them with data
- now it can just fill in the useful ones and do a 64-bit store word of 0
for the last. It should also hopefully allow a little better compression
by ssh/NX/etc. as they'll just have a string of 0's in the padding instead
of wasting time encoding random unused data.
One important lesson I learned doing this - C99 designated initializers
of unions didn't work the way I expected in gcc - you can't mix initializing
different variants of the union, which is why all the event initializations
have to initialize event.u.u.type separately from event.u.<eventtype>.* -
see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53608 for a longer explanation.
Since this touches almost every reply, event, and error in the server, I
can't claim to have fully tested it - so far nothing seems to have broken,
but hopefully review & the months of testing by a lot more people than just
me between the end of the merge window & the planned release will help shake
out any mistakes I've made.
I've also posted it as the "c99-init" branch on
git://people.freedesktop.org/~alanc/xserver.git
http://cgit.freedesktop.org/~alanc/xserver/log/?h=c99-init
Alan Coopersmith (19):
Remove unneccesary casts from WriteToClient calls
Use calloc to zero fill buffers being allocated for replies & events
Use C99 designated initializers in SendErrorToClient
Use C99 designated initializers in dix Replies
Use C99 designated initializers in Xext Replies
Use C99 designated initializers in Xinput Replies
Use C99 designated initializers in various extension Replies
Use C99 designated initializers in glx Replies
Use C99 designated initializers in xkb Replies
Use C99 designated initializers in randr Replies
Use C99 designated initializers in dmx Replies
Use C99 designated initializers in Xephyr Replies
Use C99 designated initializers in xf86 extension Replies
Use C99 designated initializers in dix Events
Use C99 designated initializers in extension Events
Use C99 designated initializers in dix registry
Set padding bytes to 0 in WriteToClient
Initialize padding bits to 0 in ErrorConnMax()
ephyrGLXQueryServerString: Stop making an unused copy of
server_string
Xext/bigreq.c | 28 +-
Xext/dpms.c | 63 ++--
Xext/geext.c | 22 +-
Xext/panoramiX.c | 151 ++++-----
Xext/panoramiXprocs.c | 45 +--
Xext/saver.c | 50 +--
Xext/security.c | 59 ++--
Xext/shape.c | 100 +++---
Xext/shm.c | 63 ++--
Xext/sync.c | 236 +++++++-------
Xext/xcmisc.c | 79 ++---
Xext/xf86bigfont.c | 42 ++-
Xext/xres.c | 127 ++++----
Xext/xselinux_ext.c | 68 ++--
Xext/xtest.c | 35 ++-
Xext/xvdisp.c | 234 +++++++-------
Xext/xvmain.c | 26 +-
Xext/xvmc.c | 190 ++++++-----
Xi/chgdctl.c | 27 +-
Xi/getbmap.c | 27 +-
Xi/getdctl.c | 14 +-
Xi/getfctl.c | 16 +-
Xi/getfocus.c | 14 +-
Xi/getkmap.c | 20 +-
Xi/getmmap.c | 23 +-
Xi/getprop.c | 16 +-
Xi/getselev.c | 18 +-
Xi/getvers.c | 32 +-
Xi/grabdev.c | 24 +-
Xi/grabdevb.c | 28 +-
Xi/grabdevk.c | 28 +-
Xi/gtmotion.c | 17 +-
Xi/listdev.c | 24 +-
Xi/opendev.c | 22 +-
Xi/queryst.c | 21 +-
Xi/setbmap.c | 30 +-
Xi/setdval.c | 39 +--
Xi/setmmap.c | 16 +-
Xi/setmode.c | 37 ++-
Xi/xigetclientpointer.c | 21 +-
Xi/xigrabdev.c | 24 +-
Xi/xipassivegrab.c | 18 +-
Xi/xiproperty.c | 160 +++++-----
Xi/xiquerydevice.c | 14 +-
Xi/xiquerypointer.c | 14 +-
Xi/xiqueryversion.c | 24 +-
Xi/xiselectev.c | 16 +-
Xi/xisetdevfocus.c | 14 +-
composite/compext.c | 67 ++--
damageext/damageext.c | 34 +-
dbe/dbe.c | 61 ++--
dix/colormap.c | 34 +-
dix/devices.c | 236 +++++++-------
dix/dispatch.c | 316 ++++++++++---------
dix/dixfonts.c | 31 +-
dix/enterleave.c | 14 +-
dix/events.c | 250 ++++++++-------
dix/extension.c | 29 +-
dix/inpututils.c | 13 +-
dix/property.c | 158 +++++-----
dix/registry.c | 9 +-
dix/selection.c | 74 ++---
dix/swaprep.c | 88 +++---
dix/swapreq.c | 2 +-
dix/window.c | 232 +++++++-------
glx/glxcmds.c | 354 +++++++++++----------
glx/glxcmdsswap.c | 18 +-
glx/glxdri2.c | 35 ++-
glx/indirect_util.c | 8 +-
glx/single2.c | 30 +-
glx/single2swap.c | 38 +--
glx/unpack.h | 12 +-
hw/dmx/dmx.c | 550 ++++++++++++++++----------------
hw/dmx/glxProxy/glxcmds.c | 292 ++++++++---------
hw/dmx/glxProxy/glxcmdsswap.c | 56 ++--
hw/dmx/glxProxy/glxsingle.c | 101 +++---
hw/dmx/glxProxy/glxvendor.c | 12 +-
hw/dmx/glxProxy/unpack.h | 12 +-
hw/kdrive/ephyr/ephyrdriext.c | 306 +++++++++---------
hw/kdrive/ephyr/ephyrglxext.c | 229 +++++++-------
hw/kdrive/ephyr/ephyrproxyext.c | 2 +-
hw/xfree86/common/xf86DGA.c | 145 +++++----
hw/xfree86/dixmods/extmod/xf86dga2.c | 200 ++++++------
hw/xfree86/dixmods/extmod/xf86vmode.c | 375 +++++++++++-----------
hw/xfree86/dri/xf86dri.c | 217 ++++++-------
hw/xfree86/dri2/dri2ext.c | 177 ++++++-----
hw/xquartz/applewm.c | 6 +-
hw/xquartz/pseudoramiX.c | 12 +-
hw/xquartz/xpr/appledri.c | 8 +-
hw/xwin/winwindowswm.c | 4 +-
include/dix.h | 4 +-
mi/miexpose.c | 13 +-
os/connection.c | 2 +-
os/io.c | 18 +-
randr/rrcrtc.c | 186 +++++------
randr/rrdispatch.c | 11 +-
randr/rrmode.c | 31 +-
randr/rroutput.c | 124 ++++----
randr/rrproperty.c | 113 ++++---
randr/rrscreen.c | 297 ++++++++++--------
randr/rrxinerama.c | 169 +++++-----
record/record.c | 61 ++--
render/render.c | 22 +-
xfixes/cursor.c | 56 ++--
xfixes/region.c | 5 +-
xfixes/select.c | 23 +-
xfixes/xfixes.c | 12 +-
xkb/xkb.c | 553 +++++++++++++++++----------------
xkb/xkbEvents.c | 57 ++--
109 files changed, 4710 insertions(+), 4310 deletions(-)
--
1.7.9.2
More information about the xorg-devel
mailing list