Created attachment 35207 [details] [review]
Reverts suspected commit

The patch will revert the suspected commit. Development was done on AMD64. Perhaps the patch changed the order of include in a an suspected way.
To speed up things, would you mind testing it?
I don't have a test environment right now.

Here is the diff:

diff --git a/configure.ac b/configure.ac
index da46c68..35c9ce8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -33,6 +33,7 @@ AC_CONFIG_AUX_DIR(.)
 # Initialize Automake
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AM_MAINTAINER_MODE
+AH_TOP([#include "xorg-server.h"])
 
 # Initialize libtool
 AC_DISABLE_STATIC
diff --git a/src/evdev.c b/src/evdev.c
index 0678edf..ccea90d 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -30,7 +30,6 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-#include <xorg-server.h>
 
 #include <X11/keysym.h>
 #include <X11/extensions/XI.h>

On Wed, Apr 21, 2010 at 07:43:31AM -0700, bugzilla-daemon@freedesktop.org wrote:
> https://bugs.freedesktop.org/show_bug.cgi?id=27768
> 
> --- Comment #1 from Gaetan Nadon <memsize@videotron.ca> 2010-04-21 07:43:30 PDT ---
> Created an attachment (id=35207)
>  View: https://bugs.freedesktop.org/attachment.cgi?id=35207
>  Review: https://bugs.freedesktop.org/review?bug=27768&attachment=35207
> 
> Reverts suspected commit

applied locally, thanks. 

Gaetan - please use "git revert" to revert patches, it provides some sort of
standardised commit message. I've done this for this patch, just keep it in
mind for next time.

fangxun - can you please confirm that the patch fixes the issue for you?

I confirm that the patch fixes this issue.

reverted, thanks for testing.


commit 539d67505cdb36018bd0e5ef01bd78939eafaadb
Author: Gaetan Nadon <memsize@videotron.ca>
Date:   Thu Apr 22 10:50:55 2010 +1000

    Revert "config: remove AH_TOP autoheader statement"

(In reply to comment #2)
> applied locally, thanks. 
> 
> Gaetan - please use "git revert" to revert patches, it provides some sort of
> standardised commit message. I've done this for this patch, just keep it in
> mind for next time.
It did not work for me. 
$~/xorg/src/driver/xf86-input-evdev$ git revert 9dbace89bee55a001e794ccf3ff36e3afeda4715
Auto-merged configure.ac
CONFLICT (content): Merge conflict in configure.ac
Auto-merged src/evdev.c
Automatic revert failed.  After resolving the conflicts,
mark the corrected paths with 'git add <paths>' or 'git rm <paths>' and commit the result.

> 
> fangxun - can you please confirm that the patch fixes the issue for you?

I checked no symbols from xorg-server.h were used in evdev (save for the version symbol needed by #include <xorgVersion.h>). I noticed for each c file that includes xorg-server.h, the library size increases by 192 bytes. That's probably not desirable. 

fangxun replied while I was writing this comment. Now that means including xorg-server.h in each c file is required even if no symbol is used. That's easy to do, blindly add it above every include config.h. But I would like to understand why.

(In reply to comment #5)
> $~/xorg/src/driver/xf86-input-evdev$ git revert
> 9dbace89bee55a001e794ccf3ff36e3afeda4715
> Auto-merged configure.ac
> CONFLICT (content): Merge conflict in configure.ac
> Auto-merged src/evdev.c
> Automatic revert failed.  After resolving the conflicts,
> mark the corrected paths with 'git add <paths>' or 'git rm <paths>' and commit
> the result.

Just fix up the conflict? :)


> I checked no symbols from xorg-server.h were used in evdev (save for the
> version symbol needed by #include <xorgVersion.h>). I noticed for each c file
> that includes xorg-server.h, the library size increases by 192 bytes. That's
> probably not desirable. 

It's probably unavoidable though:

> fangxun replied while I was writing this comment. Now that means including
> xorg-server.h in each c file is required even if no symbol is used. That's easy
> to do, blindly add it above every include config.h. But I would like to
> understand why.

It's probably due to the _XSERVER64 define, which affects the layout of various structs.

> --- Comment #5 from Gaetan Nadon <memsize@videotron.ca> 2010-04-22 05:49:58 PDT ---
> I checked no symbols from xorg-server.h were used in evdev (save for the
> version symbol needed by #include <xorgVersion.h>). I noticed for each c file
> that includes xorg-server.h, the library size increases by 192 bytes. That's
> probably not desirable. 
> 
did you strip the driver before checking that?

(In reply to comment #7)
> did you strip the driver before checking that?
No. Just an observation, not related to the issue.

On Thu, Apr 22, 2010 at 06:12:04AM -0700, bugzilla-daemon@freedesktop.org wrote:
> https://bugs.freedesktop.org/show_bug.cgi?id=27768
> 
> --- Comment #6 from Michel Dänzer <michel@daenzer.net> 2010-04-22 06:12:04 PDT ---
> (In reply to comment #5)
> > $~/xorg/src/driver/xf86-input-evdev$ git revert
> > 9dbace89bee55a001e794ccf3ff36e3afeda4715
> > Auto-merged configure.ac
> > CONFLICT (content): Merge conflict in configure.ac
> > Auto-merged src/evdev.c
> > Automatic revert failed.  After resolving the conflicts,
> > mark the corrected paths with 'git add <paths>' or 'git rm <paths>' and commit
> > the result.
> 
> Just fix up the conflict? :)

yep, that's pretty much it. just edit the file, search for "<<<<<<" and fix
up the conflict. Then git add configure.ac && git commit will give you the
right commit message. it's not different to a rebasing or merging conflict.