Ansification of X.Org code & other cleanup work

Paulo Cesar Pereira de Andrade pcpa at mandriva.com.br
Fri Oct 24 11:32:07 PDT 2008


Peter Breitenlohner wrote:
> On Mon, 20 Oct 2008, Alan Coopersmith wrote:
>
>> If someone wanted to organize a "janitorial squad" to tackle these 
>> and help
>> new people work through them to get to the point where they were 
>> ready for
>> commit access, we'd love you forever (or at least until you turn us down
>> when we then volunteer you to be the next release manager).
>>
>> [2] 41 open bugs:
>> http://bugs.freedesktop.org/buglist.cgi?keywords=janitor&product=Xorg&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED 
>>
>
> Hi Paulo,
>
> looking through that list, I noticed that your #15082 (app/xfs) has 
> already
> addressed a problem (redefined) I also noticed recently. You also mention
> two other problems:
>
> I fully agree with the prototype and declaration "BecomeDaemon (void)".
>
> To Paulo and Alan,
>
> I'm not so sure about Paulos proposal to add a prototype for 
> SnfSetFormat in
> xfs/os/config.c. I'd rather think that this function ought to be 
> declared in
> one of the headers installed by libXfont. (Is there a policy decision?)

  I think I just followed some pattern elsewhere, but really, prototypes
should be in header files, and the file actually defining the function
should also include that header.

> To all,
>
> moreover, I think one should add
>
>     if test "x$GCC" = "xyes"; then
>             GCC_WARNINGS="-Wall -Wpointer-arith -Wstrict-prototypes \
>             -Wmissing-prototypes -Wmissing-declarations \
>             -Wnested-externs -fno-strict-aliasing"
>             CFLAGS="$GCC_WARNINGS $CFLAGS"
>     fi
>
> to each and every configure.ac (if not already present) and address the
> problems uncovered this way.

  For ansification, you would also want at least -Wold-style-definition.
I also used -Wbad-function-cast and -Wdeclaration-after-statement, but
I think I only submited patches for code mixed with declarations, what
I think is now supported by Xorg (I just dislike it :-), if one needs
new variables, start a new block, otherwise pray all compilers will
have the same semantics, i.e. variables have function scope or block
scope, etc).
> To Paulo and Alan,
>
> there are additional problems with xfs:
>
> The xfs(1) manpage states (under "-daemon") that the daemon will 
> delete the
> pidfile upon exit: not true.
>
> In addition SetDaemonState in xfs/os/utils.c has code to produce an error
> message that another xfs process is already running. This never happens
> because StorePid always return either 0 or -1, and would be problematic
> because there may well be two daemons using different ports. Fact is,
> however, that when I try to start a second daemon on the same port, that
> process overwrites the original pidfile and then dies (probably 
> because it
> can't create the socket). Not very nice.

  I don't remember all the details, but if you look at
http://cvsweb.xfree86.org/cvsweb/xc/programs/xfs/
seven years ago I did several patches for xfs, to correct an exploit,
but instead of just fixing the exploit, I made a program that would
connect and feed /dev/urandom data as packets. I think I corrected
like 50 buffer overruns just by doing that, until it stayed like
one week properly handling the random data (and closing connection,
etc). But most likely it did not check all combinations, and one
could start adding random data after some initial valid data
exchange have been done; this could also be done for the X Server,
or any kind of server (and probably there should already exist
some kind of generic stress test tool for this kind of test somewhere).
> regards
> Peter Breitenlohner <peb at mppmu.mpg.de>

Paulo




More information about the xorg mailing list