diff mbox

Remove unnecessary and harmful fixincludes for Android

Message ID CACysShg_=6t=Pwr=L3AmAjJcB5Z5bUJA9WyxWDx2wRQ46A1=jA@mail.gmail.com
State New
Headers show

Commit Message

Alexander Ivchenko Aug. 4, 2014, 3:29 p.m. UTC
Hi,

The following patch disables "stdio_va_list" fix: stdio.h is already
good in Android and, since ndk gcc is indented to be used with
different Android sysroots, it is actually harmful, because without
this fix only the version of stdio.h from the sysroot the compiler was
built with will be used.

complier_h_tradcpp fix is removed with this patch. This fix was made
for Android, but it is outdated now. "linux/compiler.h" has been
changed and the fix is not being applied anymore.




Is it OK?

thanks,
Alexander

Comments

Joseph Myers Aug. 4, 2014, 8:50 p.m. UTC | #1
On Mon, 4 Aug 2014, Alexander Ivchenko wrote:

> +2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
> +
> + * inclhack.def (stdio_va_list): Disable fix for *android*.

Testing for *android* is less than ideal, because of the possibility of 
configuring a *-linux* toolchain to have multilibs using various different 
C libraries (with -mandroid being used to select the Android multilib).  
So, specifying a bypass based on some relevant text that appears in the 
header would be better.
Andrew Pinski Aug. 4, 2014, 8:54 p.m. UTC | #2
On Mon, Aug 4, 2014 at 8:29 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> Hi,
>
> The following patch disables "stdio_va_list" fix: stdio.h is already
> good in Android and, since ndk gcc is indented to be used with
> different Android sysroots, it is actually harmful, because without
> this fix only the version of stdio.h from the sysroot the compiler was
> built with will be used.

Isn't that why fixincludes is installed to run after the fact on sysroot?

Thanks,
Andrew Pinski


>
> complier_h_tradcpp fix is removed with this patch. This fix was made
> for Android, but it is outdated now. "linux/compiler.h" has been
> changed and the fix is not being applied anymore.
>
>
> diff --git a/fixincludes/ChangeLog b/fixincludes/ChangeLog
> index f7effee..19d70be 100644
> --- a/fixincludes/ChangeLog
> +++ b/fixincludes/ChangeLog
> @@ -1,3 +1,10 @@
> +2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
> +
> + * inclhack.def (stdio_va_list): Disable fix for *android*.
> + (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/fixincl.x b/fixincludes/fixincl.x
> index dd45802..8d6f1f2 100644
> --- a/fixincludes/fixincl.x
> +++ b/fixincludes/fixincl.x
> @@ -2,11 +2,11 @@
>   *
>   * DO NOT EDIT THIS FILE   (fixincl.x)
>   *
> - * It has been AutoGen-ed  Tuesday January  7, 2014 at 12:02:54 PM MET
> + * It has been AutoGen-ed  August  4, 2014 at 07:07:22 PM by AutoGen 5.12
>   * From the definitions    inclhack.def
>   * and the template file   fixincl
>   */
> -/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Jan  7 12:02:54 MET 2014
> +/* DO NOT SVN-MERGE THIS FILE, EITHER Mon Aug  4 19:07:23 MSK 2014
>   *
>   * 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 224 fixup descriptions.
> + * This file contains 223 fixup descriptions.
>   *
>   * See README for more information.
>   *
> @@ -2111,41 +2111,6 @@ int vfscanf(FILE *, const char *,
> __builtin_va_list) __asm__ (_BSD_STRING(__USER
>
>  /* * * * * * * * * * * * * * * * * * * * * * * * * *
>   *
> - *  Description of Complier_H_Tradcpp fix
> - */
> -tSCC zComplier_H_TradcppName[] =
> -     "complier_h_tradcpp";
> -
> -/*
> - *  File name selection pattern
> - */
> -tSCC zComplier_H_TradcppList[] =
> -  "linux/compiler.h\0";
> -/*
> - *  Machine/OS name selection pattern
> - */
> -#define apzComplier_H_TradcppMachs (const char**)NULL
> -
> -/*
> - *  content selection pattern - do fix if pattern found
> - */
> -tSCC zComplier_H_TradcppSelect0[] =
> -       "#define __builtin_warning\\(x, y\\.\\.\\.\\) \\(1\\)";
> -
> -#define    COMPLIER_H_TRADCPP_TEST_CT  1
> -static tTestDesc aComplier_H_TradcppTests[] = {
> -  { TT_EGREP,    zComplier_H_TradcppSelect0, (regex_t*)NULL }, };
> -
> -/*
> - *  Fix Command Arguments for Complier_H_Tradcpp
> - */
> -static const char* apzComplier_H_TradcppPatch[] = {
> -    "format",
> -    "/* __builtin_warning(x, y...) is obsolete */",
> -    (char*)NULL };
> -
> -/* * * * * * * * * * * * * * * * * * * * * * * * * *
> - *
>   *  Description of Ctrl_Quotes_Def fix
>   */
>  tSCC zCtrl_Quotes_DefName[] =
> @@ -7228,6 +7193,7 @@ tSCC zStdio_Va_ListList[] =
>   */
>  tSCC* apzStdio_Va_ListMachs[] = {
>          "*-*-solaris2.1[0-9]*",
> +        "*android*",
>          (const char*)NULL };
>
>  /*
> @@ -9187,9 +9153,9 @@ static const char* apzX11_SprintfPatch[] = {
>   *
>   *  List of all fixes
>   */
> -#define REGEX_COUNT          261
> +#define REGEX_COUNT          260
>  #define MACH_LIST_SIZE_LIMIT 187
> -#define FIX_COUNT            224
> +#define FIX_COUNT            223
>
>  /*
>   *  Enumerate the fixes
> @@ -9242,7 +9208,6 @@ typedef enum {
>      BROKEN_CABS_FIXIDX,
>      BROKEN_NAN_FIXIDX,
>      BSD_STDIO_ATTRS_CONFLICT_FIXIDX,
> -    COMPLIER_H_TRADCPP_FIXIDX,
>      CTRL_QUOTES_DEF_FIXIDX,
>      CTRL_QUOTES_USE_FIXIDX,
>      CXX_UNREADY_FIXIDX,
> @@ -9657,11 +9622,6 @@ tFixDesc fixDescList[ FIX_COUNT ] = {
>       BSD_STDIO_ATTRS_CONFLICT_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
>       aBsd_Stdio_Attrs_ConflictTests,   apzBsd_Stdio_Attrs_ConflictPatch, 0 },
>
> -  {  zComplier_H_TradcppName,    zComplier_H_TradcppList,
> -     apzComplier_H_TradcppMachs,
> -     COMPLIER_H_TRADCPP_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
> -     aComplier_H_TradcppTests,   apzComplier_H_TradcppPatch, 0 },
> -
>    {  zCtrl_Quotes_DefName,    zCtrl_Quotes_DefList,
>       apzCtrl_Quotes_DefMachs,
>       CTRL_QUOTES_DEF_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
> diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
> index 6a1136c..cd3ad2d 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:
>   *
> @@ -3727,8 +3713,9 @@ fix = {
>       * On Solaris 10, the definition in
>       * <stdio.h> is guarded appropriately by the _XPG4 feature macro;
>       * there is therefore no need for this fix there.
> +     * On Android va_list is always appropriately typedefed to __gnuc_va_list.
>       */
> -    mach = '*-*-solaris2.1[0-9]*';
> +    mach = '*-*-solaris2.1[0-9]*', '*android*';
>      not_machine = true;
>
>      /*
> 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 */
>
>
> Is it OK?
>
> thanks,
> Alexander
diff mbox

Patch

diff --git a/fixincludes/ChangeLog b/fixincludes/ChangeLog
index f7effee..19d70be 100644
--- a/fixincludes/ChangeLog
+++ b/fixincludes/ChangeLog
@@ -1,3 +1,10 @@ 
+2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
+
+ * inclhack.def (stdio_va_list): Disable fix for *android*.
+ (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/fixincl.x b/fixincludes/fixincl.x
index dd45802..8d6f1f2 100644
--- a/fixincludes/fixincl.x
+++ b/fixincludes/fixincl.x
@@ -2,11 +2,11 @@ 
  *
  * DO NOT EDIT THIS FILE   (fixincl.x)
  *
- * It has been AutoGen-ed  Tuesday January  7, 2014 at 12:02:54 PM MET
+ * It has been AutoGen-ed  August  4, 2014 at 07:07:22 PM by AutoGen 5.12
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Jan  7 12:02:54 MET 2014
+/* DO NOT SVN-MERGE THIS FILE, EITHER Mon Aug  4 19:07:23 MSK 2014
  *
  * 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 224 fixup descriptions.
+ * This file contains 223 fixup descriptions.
  *
  * See README for more information.
  *
@@ -2111,41 +2111,6 @@  int vfscanf(FILE *, const char *,
__builtin_va_list) __asm__ (_BSD_STRING(__USER

 /* * * * * * * * * * * * * * * * * * * * * * * * * *
  *
- *  Description of Complier_H_Tradcpp fix
- */
-tSCC zComplier_H_TradcppName[] =
-     "complier_h_tradcpp";
-
-/*
- *  File name selection pattern
- */
-tSCC zComplier_H_TradcppList[] =
-  "linux/compiler.h\0";
-/*
- *  Machine/OS name selection pattern
- */
-#define apzComplier_H_TradcppMachs (const char**)NULL
-
-/*
- *  content selection pattern - do fix if pattern found
- */
-tSCC zComplier_H_TradcppSelect0[] =
-       "#define __builtin_warning\\(x, y\\.\\.\\.\\) \\(1\\)";
-
-#define    COMPLIER_H_TRADCPP_TEST_CT  1
-static tTestDesc aComplier_H_TradcppTests[] = {
-  { TT_EGREP,    zComplier_H_TradcppSelect0, (regex_t*)NULL }, };
-
-/*
- *  Fix Command Arguments for Complier_H_Tradcpp
- */
-static const char* apzComplier_H_TradcppPatch[] = {
-    "format",
-    "/* __builtin_warning(x, y...) is obsolete */",
-    (char*)NULL };
-
-/* * * * * * * * * * * * * * * * * * * * * * * * * *
- *
  *  Description of Ctrl_Quotes_Def fix
  */
 tSCC zCtrl_Quotes_DefName[] =
@@ -7228,6 +7193,7 @@  tSCC zStdio_Va_ListList[] =
  */
 tSCC* apzStdio_Va_ListMachs[] = {
         "*-*-solaris2.1[0-9]*",
+        "*android*",
         (const char*)NULL };

 /*
@@ -9187,9 +9153,9 @@  static const char* apzX11_SprintfPatch[] = {
  *
  *  List of all fixes
  */
-#define REGEX_COUNT          261
+#define REGEX_COUNT          260
 #define MACH_LIST_SIZE_LIMIT 187
-#define FIX_COUNT            224
+#define FIX_COUNT            223

 /*
  *  Enumerate the fixes
@@ -9242,7 +9208,6 @@  typedef enum {
     BROKEN_CABS_FIXIDX,
     BROKEN_NAN_FIXIDX,
     BSD_STDIO_ATTRS_CONFLICT_FIXIDX,
-    COMPLIER_H_TRADCPP_FIXIDX,
     CTRL_QUOTES_DEF_FIXIDX,
     CTRL_QUOTES_USE_FIXIDX,
     CXX_UNREADY_FIXIDX,
@@ -9657,11 +9622,6 @@  tFixDesc fixDescList[ FIX_COUNT ] = {
      BSD_STDIO_ATTRS_CONFLICT_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
      aBsd_Stdio_Attrs_ConflictTests,   apzBsd_Stdio_Attrs_ConflictPatch, 0 },

-  {  zComplier_H_TradcppName,    zComplier_H_TradcppList,
-     apzComplier_H_TradcppMachs,
-     COMPLIER_H_TRADCPP_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
-     aComplier_H_TradcppTests,   apzComplier_H_TradcppPatch, 0 },
-
   {  zCtrl_Quotes_DefName,    zCtrl_Quotes_DefList,
      apzCtrl_Quotes_DefMachs,
      CTRL_QUOTES_DEF_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 6a1136c..cd3ad2d 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:
  *
@@ -3727,8 +3713,9 @@  fix = {
      * On Solaris 10, the definition in
      * <stdio.h> is guarded appropriately by the _XPG4 feature macro;
      * there is therefore no need for this fix there.
+     * On Android va_list is always appropriately typedefed to __gnuc_va_list.
      */
-    mach = '*-*-solaris2.1[0-9]*';
+    mach = '*-*-solaris2.1[0-9]*', '*android*';
     not_machine = true;

     /*
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 */