[RFC] server-private and public header separation

Mark Kettenis mark.kettenis at xs4all.nl
Sat Mar 24 05:36:45 PDT 2012


> 
> This is just a quickfire patch to show the principle, it has not been tested
> much. Plus, it's more of an idea right now, not sure if I'll find the time
> to do it.
> 
> Right now, we export virtually everything including the gory bits of every
> struct. Which causes us to break the ABI whenever we so much as look at a
> struct (see the per-device-idlecounters for example).
> 
> This patch introduces a new define __XSERVER__ that we can use in the sdk
> headers. Obviously the real integration will be a tad harder as there are
> other headers that are installed outside of include/. But the gist is that
> anything between ifdef __XSERVER__ disappears on install, (in)effectively
> hiding it.
> 
> It's going to be painful defining what goes in and what doesn't, but maybe,
> just maybe, it'll be useful in the long term.
> 
> Is that something we want?

I'd say that the __XSERVER__ define itself might be a good idea.  But
it requires discipline from developers to use it whenever appropriate
though.  There's a long tradition in (BSD) Unix to hide kernel-only
definitions in "public" headers with #ifdef _KERNEL.  Unfortunately
people tend to forget to do this quite often when they add new bits.

Howver, using __XSERVER__ to hide "private" members of structures is a
*really* bad idea.  The size of a structure is part of the ABI.
Others have already mentioned some potential pitfals.  There's another
potential issue.  By doing this you essentially create two different
structs with the same name.  When you use a debugger to debug the
server, the debugger will have to choose between those two
definitions.  It might choose the "short" one which will prevent you
from inspecting the "private" part of the structure.

* Make the structure fully opaque and provide access functions.  If
  you keep them lightweight, drivers that care about support for older
  versions of the server can easily provide their own copy of the
  access functions.

* Move all "private" members to a seperate struct and add an opaque
  pointer to this private struct to the public one.

* Rename the private members such that it is obvious they are private.
  My suggestion would be to prepend them with an underscore.  You
  could then could provide #defines to map them to their old name
  wrapped in an #ifdef __XSERVER__ block if you want to avoid making
  changes the server code.

I'm also not excited by having generated headers.  In my experience
they always cause problems with dependency tracking at some point,
especially in setups that involve NFS.  Also the BUGS section of the
BSD unifdef(1) man page makes me worried;

<http://www.freebsd.org/cgi/man.cgi?query=unifdef&apropos=0&sektion=0&manpath=FreeBSD+9.0-RELEASE&arch=default&format=html>

I have checked a couple of Linux systems I have access to, including
some systems heavily used for software development, and none of them
have unifdef(1) install.  So I seriously doubt whether it is the right
tool to use.

Is it really *that* important to prevent driver writers from shooting
themselves in the foot by defining __XSERVER__ in their code?


More information about the xorg-devel mailing list