diff mbox

Remove unnecessary and harmful fixincludes for Android

Message ID CACysShjuZS9f-Dt0oZM_SZ0j9v3VpEz4WS0fuYe+QvCnhcJARw@mail.gmail.com
State New
Headers show

Commit Message

Alexander Ivchenko Aug. 6, 2014, 11:51 a.m. UTC 
The only thing that I don't like about that, is that the user would
still have stdio.h fixed if gcc is built with sysroots older than r10.
But I guess it is not that critical :)


We still have to remove fix for compiler.h:



Bruce, I think I formally have to ask for your approval again :)

--Alexander


2014-08-06 4:31 GMT+04:00 enh <enh@google.com>:
> On Tue, Aug 5, 2014 at 5:26 PM, Bruce Korb <bkorb@gnu.org> wrote:
>> Hi,
>>
>> Lines 42 & 43 are not needed for fixincludes, but it is your choice.
>> With that change, you should not need to add that test to fixincludes
>> because __gnuc_va_list will be found within the comment and satisfy
>> the "bypass" expression.
>
> okay, i'll reword to explicitly say that it's the reference to
> __gnuc_va_list that gets us the fixincludes behavior we want (which
> should also ensure that no one "cleans up" the reference to, say,
> __builtin_va_list).
>
>> That was the long way of saying:
>>    Looks good to me.
>>
>> On Tue, Aug 5, 2014 at 5:09 PM, enh <enh@google.com> wrote:
>>> does https://android-review.googlesource.com/103445 look okay?
>>>
>>> On Tue, Aug 5, 2014 at 12:01 PM, Bruce Korb <bkorb@gnu.org> wrote:
>>>> Hi,
>>>>
>>>> On Tue, Aug 5, 2014 at 10:36 AM, enh <enh@google.com> wrote:
>>>>> you can see the current version of bionic's stdio.h here:
>>>>>
>>>>> https://android.googlesource.com/platform/bionic/+/master/libc/include/stdio.h
>>>>>
>>>>> i'm happy to add any string to the header file that makes things
>>>>> easier. if you want 'x-gcc-no-fixincludes' or whatever in there, just
>>>>> say :-)
>>>>
>>>> That would be great, but you could also add:
>>>>
>>>>    /* this file depends on __gnuc_va_list being used for va_list */
>>>>
>>>> and not bother changing fixincludes at all. :)  But either of those two
>>>> comments added to the header would be preferable to looking for "BIONIC".
>>>> Thank you!
>>>>
>>>> With one of the two changes, the patch is approved.   Thanks!

Comments

Bruce Korb Aug. 6, 2014, 2:07 p.m. UTC | #1 
Hi,

On Wed, Aug 6, 2014 at 4:51 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> We still have to remove fix for compiler.h:

Correct.  Thank you.


> Bruce, I think I formally have to ask for your approval again :)

I don't think so.  You've selected one of the changes we wrote about,
so "With one of the two changes, the patch is approved." is sufficient,
as long as you have made either of the proposed changes.

But to be clear:  Approved.  (: again :)
diff mbox

Patch

diff --git a/fixincludes/ChangeLog b/fixincludes/ChangeLog
index f7effee..b69897b 100644
--- a/fixincludes/ChangeLog
+++ b/fixincludes/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
+
+       * inclhack.def (complier_h_tradcpp): Remove.
+       * fixincl.x: Regenerate.
+       * tests/base/linux/compiler.h: Remove.
+
 2014-04-22  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

        * inclhack.def (math_exception): Bypass on *-*-solaris2.1[0-9]*.
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 6a1136c..c363c66 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -1140,20 +1140,6 @@  fix = {
 };

 /*
- *  Old Linux kernel's <compiler.h> header breaks Traditional CPP
- */
-fix = {
-    hackname  = complier_h_tradcpp;
-    files     = linux/compiler.h;
-
-    select    = "#define __builtin_warning\\(x, y\\.\\.\\.\\) \\(1\\)";
-    c_fix     = format;
-    c_fix_arg = "/* __builtin_warning(x, y...) is obsolete */";
-
-    test_text = "#define __builtin_warning(x, y...) (1)";
-};
-
-/*
  *  Fix various macros used to define ioctl numbers.
  *  The traditional syntax was:
  *
diff --git a/fixincludes/tests/base/linux/compiler.h
b/fixincludes/tests/base/linux/compiler.h
deleted file mode 100644
index 7135276..0000000
--- a/fixincludes/tests/base/linux/compiler.h
+++ /dev/null
@@ -1,14 +0,0 @@ 
-/*  DO NOT EDIT THIS FILE.
-
-    It has been auto-edited by fixincludes from:
-
-       "fixinc/tests/inc/linux/compiler.h"
-
-    This had to be done to correct non-standard usages in the
-    original, manufacturer supplied header file.  */
-
-
-
-#if defined( COMPLIER_H_TRADCPP_CHECK )
-/* __builtin_warning(x, y...) is obsolete */
-#endif  /* COMPLIER_H_TRADCPP_CHECK */