Ansification of X.Org code: A question how to proceed

[Peter Breitenlohner]

On Mon, 20 Oct 2008, Alan Coopersmith wrote:

> In general, I think everyone agrees conversion of the remaining bits
> of code that use K&R/pre-ANSI-C89 style function prototypes & declarations
> to C89 is a good thing (provided it's done correctly [1]), ....

> [1] http://invisible-island.net/ansification/index.html

Hi Alan, Adam, Julien, Paulo,

now with xorg-macros-1.2 available, I have prepared patches to convert
libICE and libSM to strict ANSI C as follows:

(1)
use xorg-macros-1.2

Use XORG_CHANGELOG for rule to generate ChangeLog from git log
Use XORG_CWARNFLAGS for compiler warning flags, leave CFLAGS to user

(2)
Activate CWARNFLAGS with lots of gcc warnings

(3)
towards ANSI C

make default error handlers and some others static

(4)
ANSI C

convert all old style function declarations

======

This should then enable and yet avoid `all' compiler (gcc) warnings

======

The problem is however, that libSM uses _IcePoMagicCookie1Proc and
_IcePaMagicCookie1Proc from libSM, but at present they are not declared in
an installed header.

A similar problem occurs with SnfSetFormat used by app/xfs and declared (but
not exported) in libXfont/src/bitmap/snfstr.h

I see two possibilities:

(A)
Declare _IceP[ao]MagicCookie1Proc in the libICE internal header
"ICElibint.h" and repeat (copy) the declaration in libSM,
analogous to Paulo's
   http://bugs.freedesktop.org/show_bug.cgi?id=15082
   http://bugs.freedesktop.org/attachment.cgi?id=15213
for SnfSetFormat and xfs

(B)
Declare _IceP[ao]MagicCookie1Proc in <X11/ICE/ICEmsg.h>, bump libICE to
1.0.5, and require 'ice >=1.0.5' for libSM

<X11/ICE/ICEmsg.h> contains already prototypes for lots of libICE
internal functions _Ice*().

(C)
There is actually a third possibility:
avoid libICE version 1.0.5, and instead test in libSM (via configure) if
_IceP[ao]MagicCookie1Proc are declared in <X11/ICE/ICEmsg.h> and otherwise
repeat their declarations.  The disadvantage is that this temporary
workaround will probably stay there forever.

=======

IMHO possibility (A), and to some extent (C), is against the spirit of [1],
but has the advantage to avoid creating a new library version.

I would prefer possibility (B), but certainly like to have your opinion on
this issue.

Solution (B) would also require to move the declaration of SnfSetFormat into
a header exported by libXfont (any good idea which one?).

====================

Once all that has been done, one can attack the real problem with libICE and
libSM.  They both contain code paths where a macro or function sets a buffer
pointer to NULL (e.g., when malloc fails or for an extremely long previous
client ID passed to libSM) but this buffer pointer is used subsequently
without any test for this condition.

====================

with best regards,
Peter Breitenlohner <peb at mppmu.mpg.de>