Bug 2901

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.