Patchwork Fix bootstrap on OpenBSD, PR48851

login
register
mail settings
Submitter Richard Guenther
Date July 4, 2011, 11:04 a.m.
Message ID <alpine.LNX.2.00.1107041301220.810@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/103088/
State New
Headers show

Comments

Richard Guenther - July 4, 2011, 11:04 a.m.
It happens that OpenBSD suffers from a bogus fixinclude that changes
its perfectly valid NULL define from (void *)0 to 0.  The fix itself
appears to be very old and is completely bogus - it replaces
(void *)0 with 0 under the assumption the former is invalid for C++ - 
which is true - but 0 is inappropriate for C which is much worse.

Thus, I propose to remove the fix altogether.  Platform maintainers
can arrange for a new fix if the platforms still need fixing (which
I seriously doubt after so many years and platform obsoletion).

This restores bootstrap on OpenBSD.

Ok for trunk and active branches?

Thanks,
Richard.

2011-07-04  Richard Guenther  <rguenther@suse.de>

	PR bootstrap/48851
	* inclhack.def (void_null): Remove bogus fix.
	* fixincl.x: Regenerated.
Bruce Korb - July 4, 2011, 12:46 p.m.
Hi Richard,

On Mon, Jul 4, 2011 at 4:04 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> It happens that OpenBSD suffers from a bogus fixinclude that changes
> its perfectly valid NULL define from (void *)0 to 0.  The fix itself
> appears to be very old and is completely bogus - it replaces
> (void *)0 with 0 under the assumption the former is invalid for C++ -
> which is true - but 0 is inappropriate for C which is much worse.
>
> Thus, I propose to remove the fix altogether.  Platform maintainers
> can arrange for a new fix if the platforms still need fixing (which
> I seriously doubt after so many years and platform obsoletion).
>
> This restores bootstrap on OpenBSD.
>
> Ok for trunk and active branches?

Sounds completely reasonable to me, but I think the platform maintainers
do need to say, "okay".  Cheers - Bruce
Richard Guenther - July 4, 2011, 12:51 p.m.
On Mon, 4 Jul 2011, Bruce Korb wrote:

> Hi Richard,
> 
> On Mon, Jul 4, 2011 at 4:04 AM, Richard Guenther <rguenther@suse.de> wrote:
> >
> > It happens that OpenBSD suffers from a bogus fixinclude that changes
> > its perfectly valid NULL define from (void *)0 to 0.  The fix itself
> > appears to be very old and is completely bogus - it replaces
> > (void *)0 with 0 under the assumption the former is invalid for C++ -
> > which is true - but 0 is inappropriate for C which is much worse.
> >
> > Thus, I propose to remove the fix altogether.  Platform maintainers
> > can arrange for a new fix if the platforms still need fixing (which
> > I seriously doubt after so many years and platform obsoletion).
> >
> > This restores bootstrap on OpenBSD.
> >
> > Ok for trunk and active branches?
> 
> Sounds completely reasonable to me, but I think the platform maintainers
> do need to say, "okay".  Cheers - Bruce

We do not have an Interix maintainer listed, that leaves David for AIX.
David, is this ok?  If not, can you please work on a better more
specific fixinclude wrapping the C++ variant inside __GNUG__?

Thanks,
Richard.
David Edelsohn - July 4, 2011, 2:31 p.m.
On Mon, Jul 4, 2011 at 8:51 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Mon, 4 Jul 2011, Bruce Korb wrote:
>
>> Hi Richard,
>>
>> On Mon, Jul 4, 2011 at 4:04 AM, Richard Guenther <rguenther@suse.de> wrote:
>> >
>> > It happens that OpenBSD suffers from a bogus fixinclude that changes
>> > its perfectly valid NULL define from (void *)0 to 0.  The fix itself
>> > appears to be very old and is completely bogus - it replaces
>> > (void *)0 with 0 under the assumption the former is invalid for C++ -
>> > which is true - but 0 is inappropriate for C which is much worse.
>> >
>> > Thus, I propose to remove the fix altogether.  Platform maintainers
>> > can arrange for a new fix if the platforms still need fixing (which
>> > I seriously doubt after so many years and platform obsoletion).
>> >
>> > This restores bootstrap on OpenBSD.
>> >
>> > Ok for trunk and active branches?
>>
>> Sounds completely reasonable to me, but I think the platform maintainers
>> do need to say, "okay".  Cheers - Bruce
>
> We do not have an Interix maintainer listed, that leaves David for AIX.
> David, is this ok?  If not, can you please work on a better more
> specific fixinclude wrapping the C++ variant inside __GNUG__?

Okay with me.

Thanks, David
Mike Stump - July 4, 2011, 7:08 p.m.
On Jul 4, 2011, at 4:04 AM, Richard Guenther wrote:
> It happens that OpenBSD suffers from a bogus fixinclude that changes
> its perfectly valid NULL define from (void *)0 to 0.  The fix itself
> appears to be very old and is completely bogus

I don't agree with the completely bogus part.  Why not replace it with:

#undef NULL
#ifdef __GNUG__
#define NULL __null
#else   /* G++ */
#ifndef __cplusplus
#define NULL ((void *)0)
#else   /* C++ */
#define NULL 0
#endif  /* C++ */
#endif  /* G++ */

?

This is C++ friendly, C friendly and modern.  It should be very safe and should work just about everywhere.

> - it replaces
> (void *)0 with 0 under the assumption the former is invalid for C++ - 
> which is true - but 0 is inappropriate for C which is much worse.

A #define to 0 is, for the C language, last I checked valid.  You may not like it, but welcome to C.

> Thus, I propose to remove the fix altogether.

Breaking all systems that are broken, isn't a good tradeoff.

Now, looking at the PR, in this case, one could add a bypass __GNUG__ to this fix, and avoid the change on OpenBSD.  This would also fix the problem.  I do not think removing the fix is a good idea.
Richard Guenther - July 5, 2011, 8:02 a.m.
On Mon, 4 Jul 2011, Mike Stump wrote:

> On Jul 4, 2011, at 4:04 AM, Richard Guenther wrote:
> > It happens that OpenBSD suffers from a bogus fixinclude that changes
> > its perfectly valid NULL define from (void *)0 to 0.  The fix itself
> > appears to be very old and is completely bogus
> 
> I don't agree with the completely bogus part.  Why not replace it with:
> 
> #undef NULL
> #ifdef __GNUG__
> #define NULL __null
> #else   /* G++ */
> #ifndef __cplusplus
> #define NULL ((void *)0)
> #else   /* C++ */
> #define NULL 0
> #endif  /* C++ */
> #endif  /* G++ */
> 
> ?

Because I don't know how to do that?

> This is C++ friendly, C friendly and modern.  It should be very safe and should work just about everywhere.
> 
> > - it replaces
> > (void *)0 with 0 under the assumption the former is invalid for C++ - 
> > which is true - but 0 is inappropriate for C which is much worse.
> 
> A #define to 0 is, for the C language, last I checked valid.  You may not like it, but welcome to C.

0 may be valid, but it doesn't work for variadic arguments.

> > Thus, I propose to remove the fix altogether.
> 
> Breaking all systems that are broken, isn't a good tradeoff.
> 
> Now, looking at the PR, in this case, one could add a bypass __GNUG__ to this fix, and avoid the change on OpenBSD.  This would also fix the problem.  I do not think removing the fix is a good idea.

Sure.

As you are objecting please take the PR and do a more proper fix then.
I don't really care about OpenBSD or AIX or Interix - I just tried to
be helpful.  So, now it's yours ;)

Thanks,
Richard.

Patch

Index: fixincludes/inclhack.def
===================================================================
--- fixincludes/inclhack.def	(revision 175800)
+++ fixincludes/inclhack.def	(working copy)
@@ -4399,32 +4399,6 @@  fix = {
 
 
 /*
- *  AIX and Interix headers define NULL to be cast to a void pointer,
- *  which is illegal in ANSI C++.
- */
-fix = {
-    hackname  = void_null;
-    files     = curses.h;
-    files     = dbm.h;
-    files     = locale.h;
-    files     = stdio.h;
-    files     = stdlib.h;
-    files     = string.h;
-    files     = time.h;
-    files     = unistd.h;
-    files     = sys/dir.h;
-    files     = sys/param.h;
-    files     = sys/types.h;
-    /* avoid changing C++ friendly NULL */
-    bypass    = __cplusplus;
-    select    = "^#[ \t]*define[ \t]+NULL[ \t]+\\(\\(void[ \t]*\\*\\)0\\)";
-    c_fix     = format;
-    c_fix_arg = "#define NULL 0";
-    test_text = "# define\tNULL \t((void *)0)  /* typed NULL */";
-};
-
-
-/*
  *  Make VxWorks header which is almost gcc ready fully gcc ready.
  */
 fix = {
Index: fixincludes/fixincl.x
===================================================================
--- fixincludes/fixincl.x	(revision 175800)
+++ fixincludes/fixincl.x	(working copy)
@@ -2,11 +2,11 @@ 
  * 
  * DO NOT EDIT THIS FILE   (fixincl.x)
  * 
- * It has been AutoGen-ed  Sunday June  5, 2011 at 09:04:54 PM CDT
+ * It has been AutoGen-ed  Monday July  4, 2011 at 12:59:38 PM CEST
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Sun Jun  5 21:04:54 CDT 2011
+/* DO NOT SVN-MERGE THIS FILE, EITHER Mon Jul  4 12:59:38 CEST 2011
  *
  * You must regenerate it.  Use the ./genfixes script.
  *
@@ -15,7 +15,7 @@ 
  * certain ANSI-incompatible system header files which are fixed to work
  * correctly with ANSI C and placed in a directory that GNU C will search.
  *
- * This file contains 211 fixup descriptions.
+ * This file contains 210 fixup descriptions.
  *
  * See README for more information.
  *
@@ -8199,48 +8199,6 @@  static const char* apzVa_I960_MacroPatch
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * *
  *
- *  Description of Void_Null fix
- */
-tSCC zVoid_NullName[] =
-     "void_null";
-
-/*
- *  File name selection pattern
- */
-tSCC zVoid_NullList[] =
-  "curses.h\0dbm.h\0locale.h\0stdio.h\0stdlib.h\0string.h\0time.h\0unistd.h\0sys/dir.h\0sys/param.h\0sys/types.h\0";
-/*
- *  Machine/OS name selection pattern
- */
-#define apzVoid_NullMachs (const char**)NULL
-
-/*
- *  content selection pattern - do fix if pattern found
- */
-tSCC zVoid_NullSelect0[] =
-       "^#[ \t]*define[ \t]+NULL[ \t]+\\(\\(void[ \t]*\\*\\)0\\)";
-
-/*
- *  content bypass pattern - skip fix if pattern found
- */
-tSCC zVoid_NullBypass0[] =
-       "__cplusplus";
-
-#define    VOID_NULL_TEST_CT  2
-static tTestDesc aVoid_NullTests[] = {
-  { TT_NEGREP,   zVoid_NullBypass0, (regex_t*)NULL },
-  { TT_EGREP,    zVoid_NullSelect0, (regex_t*)NULL }, };
-
-/*
- *  Fix Command Arguments for Void_Null
- */
-static const char* apzVoid_NullPatch[] = {
-    "format",
-    "#define NULL 0",
-    (char*)NULL };
-
-/* * * * * * * * * * * * * * * * * * * * * * * * * *
- *
  *  Description of Vxworks_Gcc_Problem fix
  */
 tSCC zVxworks_Gcc_ProblemName[] =
@@ -8591,9 +8549,9 @@  static const char* apzX11_SprintfPatch[]
  *
  *  List of all fixes
  */
-#define REGEX_COUNT          250
+#define REGEX_COUNT          248
 #define MACH_LIST_SIZE_LIMIT 181
-#define FIX_COUNT            211
+#define FIX_COUNT            210
 
 /*
  *  Enumerate the fixes
@@ -8801,7 +8759,6 @@  typedef enum {
     ULTRIX_CONST_FIXIDX,
     ULTRIX_CONST2_FIXIDX,
     VA_I960_MACRO_FIXIDX,
-    VOID_NULL_FIXIDX,
     VXWORKS_GCC_PROBLEM_FIXIDX,
     VXWORKS_NEEDS_VXTYPES_FIXIDX,
     VXWORKS_NEEDS_VXWORKS_FIXIDX,
@@ -9823,11 +9780,6 @@  tFixDesc fixDescList[ FIX_COUNT ] = {
      VA_I960_MACRO_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
      aVa_I960_MacroTests,   apzVa_I960_MacroPatch, 0 },
 
-  {  zVoid_NullName,    zVoid_NullList,
-     apzVoid_NullMachs,
-     VOID_NULL_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
-     aVoid_NullTests,   apzVoid_NullPatch, 0 },
-
   {  zVxworks_Gcc_ProblemName,    zVxworks_Gcc_ProblemList,
      apzVxworks_Gcc_ProblemMachs,
      VXWORKS_GCC_PROBLEM_TEST_CT, FD_MACH_ONLY,