I'd like to report several problems with the Xorg-6.8.2 distribution, all of them with proposed patches. Some of them have been reported to and fixed in XFree86 about a year ago! Among these problems: If XOrg (xdm) is compiled with IPv6 support, but the (linux) kernel doen't support IPv6, some code cannot possibly work (and indeed fails). PLEASE: tell me how to submit patches.
To submit a patch, open a bug in bugzilla as you've done here, then go back and view the bug and click on "Create a New Attachment" to attach the patch to the bug report.
Oh, and just to be clear - patches must be either your own work, or submitted with the permission of the original author. Do not send us patches taken from the XFree86 tree without confirming they are not covered by the new XFree86 license.
Created attachment 2317 [details] [review] Fix some of the IPv6 problems in xdm
Created attachment 2318 [details] [review] build Speedo fonts by default It is rather strange to have Speedo fonts both in the default FontPath and in the recommended FontPath and at the same time by default disable support for and building of the Speedo fonts.
Created attachment 2319 [details] [review] Don't use "gcc -ansi -pedantic" Since many files now use non-ANSI constructs you shouldn't use "gcc -ansi -pedantic".
Created attachment 2320 [details] [review] non-portable whitespace According to what I was told by the gcc and glibc people leading whitespace in preprocessor directives is allowed by more recent C/C++ specs.
Created attachment 2321 [details] [review] sscanf bug This is definitely a bug (found via the gcc warning "format argument is not a pointer")
Created attachment 2322 [details] [review] fix "make -n install" failure Without this patch 'make -n install' will almost necessarily fail.
Created attachment 2323 [details] [review] Don't build Xdarwin on linux You certainly don't want to build and install the XDarwin server on non-Darwin architectures
Created attachment 2324 [details] [review] Avoid (non-identical) redefinition of preprocessor macros This patch removes 462 pairs of gcc-3.4.3 warnings: "xxx" redefined this is the location of the previous definition
Created attachment 2325 [details] [review] Avoid various gcc-warnings Avoid 38 gcc-3.4.2 warnings: suggest parentheses around assignment used as truth value and similar
Created attachment 2326 [details] [review] More gcc warnings This patch removes these 2 gcc-3.4.3 warnings: missing braces around initializer implicit declaration of function `XpOidTrayMediumListHasTray'
peter: Is it possible to file seperate bugs (and CC: me) for these issues, please (one bug per issue) ?
Created attachment 2327 [details] [review] function prototypes This patch removes 3 of the gcc-3.4.3 warnings: implicit declaration of function `xxx' There are many more such warnings, at least one of them almost certainly a bug (see comment in the attached patch)/
Created attachment 2328 At one time the was a prototype for HALSetDisplayStart in the binding.h file provided by Matrox. Why has this disappeared again?
Created attachment 2329 At one time the was a prototype for HALSetDisplayStart in the binding.h file provided by Matrox. Why has this disappeared again?
Created attachment 2330 [details] [review] Avoid duplicate make rule This patch removes 10 pairs of GNU make-3.80 warnings: overriding commands for target `nsc._man' ignoring old commands for target `nsc._man'
Created attachment 2331 [details] [review] Application defaults ending with "<backslash><space>" Cpp from gcc >=3.3 treats "<backslash><space><newline>" in the same way as "<backslash><newline>" (possibly with a warning). Moreover gcc-3.3 and gcc-3.4 behave slightly different. It is therefore extremely difficult, if not impossible to define application defaults ending with "<backslash><space>" in a portable way. Things get even worse when cpp is used to generate XOrgCfg.ad from XOrgCfg.cpp.
Created attachment 2332 [details] [review] gcc warnings: declared static but never defined This patch avoids 79 gcc-3.4.3 warnings 'xxx' declared `static' but never defined mostly due to including "ftfuncs.h" with the declaration of static functions defined in "ftfuncs.c".
peter: this is excellent work, but our typical commit process is to use one bug number per issue so that we can track each issue individually.
I'll start working through these.
Comment on attachment 2318 [details] [review] build Speedo fonts by default The correct fix is to remove the speedo fonts from the path. Speedo support was disabled on purpose, and this patch simply undoes that, so I'm rejecting it. (If it was a separate bug I'd mark WONTFIX, but since these are all lumped together, I'll just mark the patch obsolete.)
First patch ("Fix some of the IPv6 problems in xdm") committed to CVS head: CVSROOT: /cvs/xorg Module name: xc Changes by: alanc@gabe.freedesktop.org 05/07/05 11:52:33 Log message: 2005-07-05 Alan Coopersmith <alan.coopersmith@sun.com> * programs/xdm/auth.c: * programs/xdm/socket.c: * programs/xdm/xdmcp.c: Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2317 <https://bugs.freedesktop.org/attachment.cgi?id=2317> This patch fixes several problems arising if xdm is compiled with IPv6 support, but the (linux) kernel does not support IPv6. (Peter Breitenlohner) Modified files: ./: ChangeLog xc/programs/xdm/: auth.c socket.c xdmcp.c Revision Changes Path 1.1087 +12 -0 xc/ChangeLog 1.3 +3 -3 xc/programs/xdm/auth.c 1.4 +10 -8 xc/programs/xdm/socket.c 1.5 +8 -5 xc/programs/xdm/xdmcp.c
Third ("Don't use gcc -ansi -pedantic") & fourth ("non-portable whitespace warning in makedepend") patches commited to CVS head: Log message: 2005-07-05 Alan Coopersmith <alan.coopersmith@sun.com> * config/cf/xorg.cf: * config/cf/xfree86.cf: Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2319 <https://bugs.freedesktop.org/attachment.cgi?id=2319> Since many files now use non-ANSI constructs ('long long', substructure initializers, implicit signedness conversions, ...) you shouldn't use "gcc -ansi -pedantic". On our ix86-linux-gnu system this change reduces the number of gcc-3.4.3 warnings from originally 3034 to 1460! (Peter Breitenlohner) Modified files: xc/config/cf/: xorg.cf xfree86.cf ./: ChangeLog Revision Changes Path 1.49 +1 -1 xc/config/cf/xorg.cf 1.13 +1 -1 xc/config/cf/xfree86.cf 1.1089 +13 -0 xc/ChangeLog Log message: 2005-07-05 Alan Coopersmith <alan.coopersmith@sun.com> * config/makedepend/main.c: Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2320 <https://bugs.freedesktop.org/attachment.cgi?id=2320> Remove warning about whitespace in pre-processor defines. (Peter Breitenlohner) Modified files: ./: ChangeLog xc/config/makedepend/: main.c Revision Changes Path 1.1088 +9 -0 xc/ChangeLog 1.4 +0 -4 xc/config/makedepend/main.c
Normally, I'd say "report it upstream" for the packages in extras, but it appears extras/ttf2pt1 is already forked from upstream and modified quite a bit for Xprint, so I've gone ahead and applied Patch #5 ("sscanf bug"): CVSROOT: /cvs/xorg Module name: xc Changes by: alanc@gabe.freedesktop.org 05/07/05 12:29:29 Log message: 2005-07-05 Alan Coopersmith <alan.coopersmith@sun.com> * extras/ttf2pt1/ttf2pt1.c: Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2321 <https://bugs.freedesktop.org/attachment.cgi?id=2321> Arguments to sscanf need to be pointers (Peter Breitenlohner) Modified files: ./: ChangeLog xc/extras/ttf2pt1/: ttf2pt1.c Revision Changes Path 1.1090 +8 -0 xc/ChangeLog 1.3 +1 -1 xc/extras/ttf2pt1/ttf2pt1.c
Comment on attachment 2329 Marking attachment #2329 as obsolete since it's a duplicate of #2328.
Comment on attachment 2328 Marking attachment #2328 as obsolete - it's been refiled as bug #3712 so it can be tracked separately.
Patches 9, 10, & 14 committed to CVS head: CVSROOT: /cvs/xorg Module name: xc Changes by: alanc@gabe.freedesktop.org 05/07/06 19:47:06 Log message: 2005-07-06 Alan Coopersmith <alan.coopersmith@sun.com> * extras/ttf2pt1/ft.c: * extras/ttf2pt1/pt1.c: * lib/XprintAppUtil/xpapputil.c: * programs/Xserver/Xext/xevie.c: * programs/Xserver/Xprint/Init.c: * programs/Xserver/Xprint/Oid.c: * programs/Xserver/Xprint/ps/PsFonts.c: Bug #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2325 <https://bugs.freedesktop.org/attachment.cgi?id=2325> Avoid 38 gcc-3.4.2 warnings: suggest parentheses around assignment used as truth value suggest explicit braces to avoid ambiguous `else' suggest parentheses around && within || suggest parentheses around arithmetic in operand of ^ "/*" within comment (Peter Breitenlohner) * programs/Xserver/Xprint/AttrValid.c: * programs/Xserver/Xprint/Oid.h: Bug #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2326 <https://bugs.freedesktop.org/attachment.cgi?id=2326> This patch removes these 2 gcc-3.4.3 warnings: missing braces around initializer implicit declaration of function `XpOidTrayMediumListHasTray' (Peter Breitenlohner) Modified files: ./: ChangeLog xc/extras/ttf2pt1/: ft.c pt1.c xc/lib/XprintAppUtil/: xpapputil.c xc/programs/Xserver/Xext/: xevie.c xc/programs/Xserver/Xprint/: AttrValid.c Init.c Oid.c Oid.h xc/programs/Xserver/Xprint/ps/: PsFonts.c Revision Changes Path 1.1096 +28 -0 xc/ChangeLog 1.3 +1 -1 xc/extras/ttf2pt1/ft.c 1.3 +32 -32 xc/extras/ttf2pt1/pt1.c 1.4 +1 -1 xc/lib/XprintAppUtil/xpapputil.c 1.10 +2 -2 xc/programs/Xserver/Xext/xevie.c 1.4 +1 -1 xc/programs/Xserver/Xprint/AttrValid.c 1.17 +6 -6 xc/programs/Xserver/Xprint/Init.c 1.5 +7 -1 xc/programs/Xserver/Xprint/Oid.c 1.5 +1 -0 xc/programs/Xserver/Xprint/Oid.h 1.5 +1 -1 xc/programs/Xserver/Xprint/ps/PsFonts.c CVSROOT: /cvs/xorg Module name: xc Changes by: alanc@gabe.freedesktop.org 05/07/06 19:53:58 Log message: 2005-07-06 Alan Coopersmith <alan.coopersmith@sun.com> * programs/Xserver/hw/xfree86/drivers/nsc/Imakefile: Bug #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2330 <https://bugs.freedesktop.org/attachment.cgi?id=2330> Remove CppManTarget since it's already included in InstallModuleManPage expansion - removes 10 pairs of warnings from GNU make about duplicate Makefile targets (Peter Breitenlohner) Modified files: xc/programs/Xserver/hw/xfree86/drivers/nsc/: Imakefile ./: ChangeLog Revision Changes Path 1.5 +0 -1 xc/programs/Xserver/hw/xfree86/drivers/nsc/Imakefile 1.1097 +9 -0 xc/ChangeLog
Comment on attachment 2324 [details] [review] Avoid (non-identical) redefinition of preprocessor macros Marking patch #8 as obsolete. I've commited a fix for the xorgcfg warnings to CVS head, and moved the ATI register warnings to a new bug, bug #3718, where the ATI driver & DRI maintainers can work it out.
Patch #16 applied to CVS head: CVSROOT: /cvs/xorg Module name: xc Changes by: alanc at gabe.freedesktop.org 05/07/07 07:59:48 Log message: 2005-07-07 Alan Coopersmith <alan.coopersmith at sun.com> * extras/ttf2pt1/ft.c: * extras/ttf2pt1/pt1.c: * extras/ttf2pt1/ttf2pt1.c: * lib/font/FreeType/ftfuncs.h: * programs/Xserver/Xprint/ps/PsFTFonts.c: * programs/Xserver/Xprint/ps/psout_ft.c: * programs/Xserver/Xprint/ps/psout_ftpstype1.c: * programs/Xserver/Xprint/ps/psout_ftpstype3.c: Bug #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2332 <https://bugs.freedesktop.org/attachment.cgi?id=2332> This patch avoids 79 gcc-3.4.3 warnings 'xxx' declared `static' but never defined mostly due to including "ftfuncs.h" with the declaration of static functions defined in "ftfuncs.c". (Peter Breitenlohner) Modified files: ./: ChangeLog xc/extras/ttf2pt1/: ft.c pt1.c ttf2pt1.c xc/lib/font/FreeType/: ftfuncs.h xc/programs/Xserver/Xprint/ps/: PsFTFonts.c psout_ft.c psout_ftpstype1.c psout_ftpstype3.c Revision Changes Path 1.1099 +17 -0 xc/ChangeLog 1.4 +1 -0 xc/extras/ttf2pt1/ft.c 1.4 +2 -0 xc/extras/ttf2pt1/pt1.c 1.4 +1 -0 xc/extras/ttf2pt1/ttf2pt1.c 1.4 +4 -0 xc/lib/font/FreeType/ftfuncs.h 1.4 +1 -0 xc/programs/Xserver/Xprint/ps/PsFTFonts.c 1.7 +1 -0 xc/programs/Xserver/Xprint/ps/psout_ft.c 1.7 +1 -0 xc/programs/Xserver/Xprint/ps/psout_ftpstype1.c 1.7 +1 -0 xc/programs/Xserver/Xprint/ps/psout_ftpstype3.c
BTW, a number of the patches contain comments like this: NOTE: XFree86-4.4.0 produced very few such warnings and XFree86-4.5.0 doesn't produce any. Wouldn't it be nice to keep XOrg and XFree86 more in sync in these respects? Yes, we would love to keep them more in sync. If everyone reporting bugs reported them to both, it would be closer, but as long as XFree86 4.4.0 and later are covered by the XFree86 1.1 license, we cannot bring in most changes directly from them. (The X.Org license is less restrictive, so they are still free to bring in changes from X.Org, as they've done many times in the past.)
Comment on attachment 2327 [details] [review] function prototypes Marking patch #11 as obsolete. The first fix is against DRI, which is maintained outside the Xorg tree and needs to be taken up with the DRI maintainers. The next two are against code in Xprint that's no longer present, and the replacement has proper prototypes. That leaves only the last one, for XdmAuthenticationInit, which I've gone ahead and committed to CVS head.
I don't see any reason to apply patch #7 ("Don't build Xdarwin on linux") - it's already not built on linux because only xc/config/cf/darwin.cf defines XDarwinServer. Patch #6 I'm also unsure about - it allows make -n install but removes warnings from make install when real failure has occurred. That leaves only patch #15 to be committed before this jumbo bug can be closed. (Thanks for all the patches, but please file them in separate bugs in the future to allow easier tracking of which ones have or have not been integrated, and easier splitting of the work between the various people working on the tree.)
(In reply to comment #33) > I don't see any reason to apply patch #7 ("Don't build Xdarwin on linux") - it's > already not built on linux because only xc/config/cf/darwin.cf defines > XDarwinServer. Fact is, without that patch Xdarwin was built on Linux, although not installed. > Patch #6 I'm also unsure about - it allows make -n install but removes warnings > from make install when real failure has occurred. (1) I think "make -n install" certainly must not fail (any worse than "make install" would). (2) I think real failures will still show up, although the error messages / warnings wight be somewhat obscure.
Patch #15 (Application defaults ending with "<backslash><space>") committed to CVS head: 2005-08-17 Alan Coopersmith <alan.coopersmith@sun.com> * programs/Xserver/hw/xfree86/xf86cfg/XOrgCfg.cpp Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2331 <https://bugs.freedesktop.org/attachment.cgi?id=2331> Lines ending in <backslash><space> break with cpp from gcc >= 3.3 (Peter Breitenlohner)
Patch #6 committed to CVS head: 2005-08-18 Alan Coopersmith <alan.coopersmith@sun.com> * config/cf/Imake.rules: Bugzilla #2901 <https://bugs.freedesktop.org/show_bug.cgi?id=2901> Patch #2322 <https://bugs.freedesktop.org/attachment.cgi?id=2322> Fix "make -n install" failure (Peter Breitenlohner) That was the last one, so I'm closing this bug as fixed.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.