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

Paulo César Pereira de Andrade pcpa at
Sun Dec 7 10:45:19 PST 2008

  Hi Peter,

>> 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]
> 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)
> 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
> for SnfSetFormat and xfs

  This is arguably the worst option :-)  Actually, prototypes
declared in C sources should be moved to the proper headers.

> (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*().

  This is I believe, the best option. And while at it, change the
calls to iceauth.c:binaryEqual() to memcmp() (or maybe it is done
that way to avoid someone somehow LD_PRELOAD'ing memcp ?, but then,
one could just LD_PRELOAD libICE ...)

> (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?).

  X11/fonts/fontmisc.h appears to be most appropriate place, with
fontutil.h second, but xfs/os/config.c already include
X11/fonts/fontutil.h (but maybe it is also including fontmisc.h

> ====================
> 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>


More information about the xorg mailing list