Patchwork [fixincludes] Fix NULL on AIX

login
register
mail settings
Submitter David Edelsohn
Date April 30, 2013, 2:40 p.m.
Message ID <CAGWvny=3PiUFQQwNKpAxTX1G1tXB8HfoOUwkmRpvdi2WyKhAXg@mail.gmail.com>
Download mbox | patch
Permalink /patch/240632/
State New
Headers show

Comments

David Edelsohn - April 30, 2013, 2:40 p.m.
AIX headers do not have a complete definition of NULL and the GCC
build complains about "missing sentinel".

The appended patch substitutes a more complete definition of NULL that
satisfies GCC/G++.

I also updated the void_null fix, which was discussed in 2011 to
substitute a more complete definition of NULL.  void_null no longer
triggers on AIX because of the "__cplusplus" bypass, which appears in
AIX header files.  I can omit void_null from the patch or remove the
fix completely if it no longer is applicable to any target.

(Yes, I will update fixinclude tests, but I first want agreement on
the fix itself.)

Bootstrapped on powerpc-ibm-aix7.1.0.0.

Thanks, David

        * inclhack.def (aix_null): New.
        (void_null): Substitute newer definition of NULL.
        * fixincl.x: Regenerate.
Bruce Korb - April 30, 2013, 3:28 p.m.
Hi David,

Looks good to me, but for a small nit:

On Tue, Apr 30, 2013 at 7:40 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> +    c_fix_arg = "#ifndef NULL\n"
> +               "#ifdef __cplusplus\n"
> +               "#ifdef __GNUG__\n"
> +               "#define NULL\t__null\n"
> +               "#else\t /* ! __GNUG__  */\n"
> +               "#define NULL\t0L\n"
> +               "#endif\t /* __GNUG__  */\n"
> +               "#else\t /* ! __cplusplus  */\n"
> +               "#define NULL\t((void *)0)\n"
> +               "#endif\t /* __cplusplus  */\n"
> +               "#endif\t /* !NULL  */";

I'd really prefer a "here string":

c-fix-arg = <<- _EOF_
        #ifndef NULL
        #ifdef __cplusplus
        #ifdef __GNUG__
        #define NULL    __null
        #else   /* ! __GNUG__  */
        #define NULL    0L
        #endif  /* __GNUG__  */
        #else   /* ! __cplusplus  */
        #define NULL    ((void *)0)
        #endif  /* __cplusplus  */
        #endif  /* !NULL  */
        _EOF_;

Unless there is some reason you want to emphasize tabs with the "\t" escapes.
I think the "here string" is easier to read.
David Edelsohn - April 30, 2013, 4:42 p.m.
On Tue, Apr 30, 2013 at 11:28 AM, Bruce Korb <bkorb@gnu.org> wrote:
> Hi David,
>
> Looks good to me, but for a small nit:

> Unless there is some reason you want to emphasize tabs with the "\t" escapes.
> I think the "here string" is easier to read.

I coped c_fix_arg from openbsd_null_definition fix. I don't know if
two different formats for similar fixes would be confusing, but I do
not have a problem using the EOF version.

Do you have any comment about the change to void_null fix?

Thanks, David
Bruce Korb - April 30, 2013, 5:09 p.m.
Hi,

On Tue, Apr 30, 2013 at 9:42 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> I coped c_fix_arg from openbsd_null_definition fix. I don't know if
> two different formats for similar fixes would be confusing, but I do
> not have a problem using the EOF version.

I haven't reviewed all patches and I don't always raise nits.

> Do you have any comment about the change to void_null fix?

By analogy, I'd prefer the EOF version there, too.  But ultimately,
it works and is not exactly illegible, so I have no serious objection
to the fix.  It would be nice if it were combinable, but doing so is
tricky.  I'm guessing that the fix on AIX is separated because the
headers contain "__cplusplus" and that is used as an exclusion test
for the "void_null" fix?  Otherwise this:

select    = "^#[ \t]*define[ \t]+NULL[ \t]+("
                    "\\(\\(void[ \t]*\\*\\)0\\)"   "|"   "\\(*0L*\\)*"   ")";

would allow void_null to work for you -- perhaps by using %1 somewhere
in the replacement "c-fix-arg".

Cheers - Bruce
David Edelsohn - April 30, 2013, 5:43 p.m.
On Tue, Apr 30, 2013 at 1:09 PM, Bruce Korb <bkorb@gnu.org> wrote:

> By analogy, I'd prefer the EOF version there, too.  But ultimately,
> it works and is not exactly illegible, so I have no serious objection
> to the fix.  It would be nice if it were combinable, but doing so is
> tricky.  I'm guessing that the fix on AIX is separated because the
> headers contain "__cplusplus" and that is used as an exclusion test
> for the "void_null" fix?  Otherwise this:
>
> select    = "^#[ \t]*define[ \t]+NULL[ \t]+("
>                     "\\(\\(void[ \t]*\\*\\)0\\)"   "|"   "\\(*0L*\\)*"   ")";
>
> would allow void_null to work for you -- perhaps by using %1 somewhere
> in the replacement "c-fix-arg".

Yes, I created a separate fix because the void_null fix has additional
selection details that I did not want to change and possibly break
other platforms.

Thanks, David

Patch

Index: inclhack.def
===================================================================
--- inclhack.def        (revision 198437)
+++ inclhack.def        (working copy)
@@ -617,6 +617,31 @@ 
 };

 /*
+ *  Fix AIX definition of NULL for G++.
+ */
+fix = {
+    hackname  = aix_null;
+    mach      = "*-*-aix*";
+    files     = curses.h, dbm.h, locale.h, stdio.h, stdlib.h, string.h,
+    time.h, unistd.h, wchar.h, sys/dir.h, sys/param.h, sys/types.h;
+    bypass    = __null;
+    select    = "#define[ \t]+NULL[ \t]+\\(*0L*\\)*";
+    c_fix     = format;
+    c_fix_arg = "#ifndef NULL\n"
+               "#ifdef __cplusplus\n"
+               "#ifdef __GNUG__\n"
+               "#define NULL\t__null\n"
+               "#else\t /* ! __GNUG__  */\n"
+               "#define NULL\t0L\n"
+               "#endif\t /* __GNUG__  */\n"
+               "#else\t /* ! __cplusplus  */\n"
+               "#define NULL\t((void *)0)\n"
+               "#endif\t /* __cplusplus  */\n"
+               "#endif\t /* !NULL  */";
+    test_text = "# define\tNULL \t(0L)  /* typed NULL */";
+};
+
+/*
  *  pthread.h on AIX defines PTHREAD_ONCE_INIT, PTHREAD_MUTEX_INITIALIZER,
  *  PTHREAD_COND_INITIALIZER and PTHREAD_RWLOCK_INITIALIZER without enough
  *  braces.
@@ -4552,23 +4577,24 @@ 
  */
 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;
+    files     = curses.h, dbm.h, locale.h, stdio.h, stdlib.h, string.h,
+    time.h, unistd.h, sys/dir.h, sys/param.h, sys/types.h;
     /* avoid changing C++ friendly NULL */
     bypass    = __cplusplus;
     bypass    = __null;
     select    = "^#[ \t]*define[ \t]+NULL[ \t]+\\(\\(void[ \t]*\\*\\)0\\)";
     c_fix     = format;
-    c_fix_arg = "#define NULL 0";
+    c_fix_arg = "#ifndef NULL\n"
+               "#ifdef __cplusplus\n"
+               "#ifdef __GNUG__\n"
+               "#define NULL\t__null\n"
+               "#else\t /* ! __GNUG__  */\n"
+               "#define NULL\t0L\n"
+               "#endif\t /* __GNUG__  */\n"
+               "#else\t /* ! __cplusplus  */\n"
+               "#define NULL\t((void *)0)\n"
+               "#endif\t /* __cplusplus  */\n"
+               "#endif\t /* !NULL  */";
     test_text = "# define\tNULL \t((void *)0)  /* typed NULL */";
 };