| Message ID | alpine.LNX.2.00.1107041301220.810@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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.
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.
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,