xserver janitorial work

[Keith Packard]

On Wed, 2006-06-07 at 16:15 -0700, Greg KH wrote:

> > - parameters that get promoted to int or double in K&R style
> > (they should ihmo be declared as int or double in "ansi" declaration, in 
> > order to be compatible with old code)
> 
> How would this happen?  With variables that have no explicit "type"?

You clearly aren't used to the nightmare known as K&R C.

foo (float x) vs foo (x) float x;

In the ANSI case, the type of 'x' is float, and in the K&R case, the
type of 'x' is passed as a double and then converted to a float as it is
used. That was trivial on the PDP11 and VAX where float and double had
the same data in the first 32-bits (exponents were the same length).

> The compiler _should_ catch this now with a proper function prototype
> (I'm also trying to clean up these and the ever popular "just stick an
> extern foo(int); in this other file" issues so this is caught at build
> time.)

It's only a warning these days, and with the number of warnings emitted
by the compiler when faced with X code, they're generally lost in the
noise. Yes, the goal is to get to zero warnings, but as an intermediate
step, we must be careful to not break things even while most of us
continue to ignore huge piles of warnings coming out of the compiler
from the legacy code. This is why there hasn't been a wholesale
conversion of function declarations and instead we've done it a bit at a
time, to not get careless.

> 'git bisect' will help out with that :)

Actually, just getting the tinderbox going again will find some of these
right away.

> But yes, I understand the issues and will be very careful to try to not
> mess up.

That's all we ask; it's a huge, fragile code base that all runs as root
with most of the delicate parts of your system mmaped in. Treat it like
kernel code that hasn't been well maintained, and you'll get a good idea
of our caution.

> > Another point that is worth noticing is that the X server uses quite a 
> > lot of pointer to functions of incomplete types, with several different 
> > actual function types. Converting this code generally means adding 
> > unions or other rather ugly stuff. These should be reviewed on a case by 
> > case basis to find the less ugly way to handle it.

There aren't too many of these any longer; the biggest user was always
stubs for various GC ops and the like.

> I guess I'm just used to "cleaner" .c code, and IMHO, no #ifdefs in the
> middle of a function is a good idea.

It's an interesting position, but I'm not sure it's more or less clear
in all cases.

> It looks like I have enough other good "real" cleanups to do first so
> I'll hold off on doing this for now.

Yeah, reducing the huge number of warnings from the compiler would be a
great assistance in making the code more maintainable; right now, we end
up ignoring a lot of valuable compiler advice as a result.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20060607/0a3da7ec/attachment.pgp>