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

[Paulo César Pereira de Andrade]

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

  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
indirectly).


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

Paulo