Re: [PATCH] fixincludes: Update darwin_flt_eval_method for macOS 14

[Bruce Korb via Gcc-patches]

Looks reasonable to me!

On 8/16/23 12:20, Rainer Orth wrote:

On macOS 14, a guard in  changed:

-- MacOSX13.3.sdk/usr/include/math.h2023-04-19 01:54:44
+++ MacOSX14.0.sdk/usr/include/math.h   2023-08-01 08:42:43
@@ -22,0 +23 @@
+
@@ -43 +44 @@
-#if __FLT_EVAL_METHOD__ == 0
+#if __FLT_EVAL_METHOD__ == 0 || __FLT_EVAL_METHOD__ == -1
@@ -49 +50 @@
-#elif __FLT_EVAL_METHOD__ == 2 || __FLT_EVAL_METHOD__ == -1
+#elif __FLT_EVAL_METHOD__ == 2

Therefore the darwin_flt_eval_method fixincludes fix doesn't match any
longer, leading to a large number of testsuite failures like

/private/var/gcc/regression/master/14-gcc/build/gcc/include-fixed/math.h:69:5: error: 
#error "Unsupported value of __FLT_EVAL_METHOD__."

where __FLT_EVAL_METHOD__ = 16.

This patch adjusts the fix to allow for both forms.

Tested with make check in fixincludes on x86_64-apple-darwin23.0.0 and
verifying that  has indeed been fixed as expected.

Ok for trunk?

Rainer

Re: [PATCH] fixincludes: don't assume getcwd() can handle NULL argument

[Bruce Korb via Gcc-patches]

On 11/10/21 4:22 AM, Xi Ruoyao wrote:

Isn't this warning actually a glibc bug
?

However we can't assume the libc we are using is Glibc.  Even if the
libc supports getcwd() with NULL argument, we are still leaking memory.
You could also free the memory by calling exit(2). Something is pretty 
wrong if fixincludes cannot access a file it was told to process. So, 
technically, you're right. Practically, no difference. But it's fine by 
me to make the change. It does avoid a bug in a certain version of a 
certain library.

Re: [PATCH] fixincludes: fix portability issues about getcwd() [PR21283, PR80047]

[Bruce Korb via Gcc-patches]

If you are going to be excruciatingly, painfully correct, free() is 
going to be unhappy about freeing a static string in the event getcwd() 
fails for some inexplicable reason. I'd replace the free() + return with 
a call to exit. Maybe even:


   if (VERY_UNLIKELY (access (pz_curr_file, R_OK) != 0)) abort()

On 11/11/21 8:33 AM, Xi Ruoyao wrote:

---
  fixincludes/fixincl.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..1580c67efec 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -1353,9 +1353,18 @@ process (void)
if (access (pz_curr_file, R_OK) != 0)
  {
int erno = errno;
+  char *buf = NULL;
+  const char *cwd = NULL;
+  for (size_t size = 256; !cwd; size += size)
+   {
+ buf = xrealloc (buf, size);
+ cwd = getcwd (buf, size);
+ if (!cwd && errno != ERANGE)
+   cwd = "the working directory";
+   }
fprintf (stderr, "Cannot access %s from %s\n\terror %d (%s)\n",
-   pz_curr_file, getcwd ((char *) NULL, MAXPATHLEN),
-   erno, xstrerror (erno));
+  pz_curr_file, cwd, erno, xstrerror (erno));
+ free (buf); return;
  }
  

Re: [PATCH] fixincludes: simplify handling for access() failure [PR21283, PR80047]

[Bruce Korb via Gcc-patches]

Perfect.

On 11/12/21 1:58 PM, Xi Ruoyao wrote:

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..ee57fbf61b4 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -1352,11 +1352,10 @@ process (void)
  
if (access (pz_curr_file, R_OK) != 0)

  {
-  int erno = errno;
-  fprintf (stderr, "Cannot access %s from %s\n\terror %d (%s)\n",
-   pz_curr_file, getcwd ((char *) NULL, MAXPATHLEN),
-   erno, xstrerror (erno));
-  return;
+  /* Some really strange error happened. */
+  fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file,
+  xstrerror (errno));
+  abort();
  }
  
pz_curr_data = load_file (pz_curr_file);

Vim swap files not ignored

[Bruce Korb via Gcc-patches]

Hi,
I don't have the keys for write access anymore. This ought to be 
applied. Odd that it never has been. :)diff --git a/.gitignore b/.gitignore
index 14ee0325176..021a8c74185 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,6 +6,7 @@
 *~
 .#*
 *#
+.*.swp
 
 *.flt
 *.gmo

Re: [PATCH v2] fixinc: don't "fix" machine names in __has_include(...) [PR91085]

[Bruce Korb via Gcc-patches]

Hi Xi,

On 6/27/21 11:07 PM, Xi Ruoyao wrote:

diff --git a/fixincludes/fixfixes.c b/fixincludes/fixfixes.c
index 5b23a8b640d..147cba716c7 100644
--- a/fixincludes/fixfixes.c
+++ b/fixincludes/fixfixes.c
@@ -524,7 +524,7 @@ FIX_PROC_HEAD( machine_name_fix )
/* If the 'name_pat' matches in between base and limit, we have
   a bogon.  It is not worth the hassle of excluding comments
   because comments on #if/#ifdef lines are rare, and strings on
- such lines are illegal.
+ such lines are only legal in a "__has_include" directive.
  
   REG_NOTBOL means 'base' is not at the beginning of a line, which

   shouldn't matter since the name_re has no ^ anchor, but let's
@@ -544,6 +544,31 @@ FIX_PROC_HEAD( machine_name_fix )
  break;
  
p = base + match[0].rm_so;


This function is already 90 lines long. This would be better in a function.


+
+  /* Check if the match is in __has_include(...) (PR 91085). */
+  for (q = base; q < p; q++)
+if (!strncmp (q, "__has_include", 13))
+  {
+r = q + 13;
+while (r < p && ISSPACE (*r))
+  r++;
+
+/* "__has_include" may appear as "defined(__has_include)",
+   search for the next appearance then.  */
+if (*r != '(')
+  continue;
+
+/* To avoid too much complexity, just hope there is never a
+   ')' in a header name.  */
+while (r < limit && *r != ')')
+  r++;


strchr()? I'd use strchr() to find the start of "__has_include" as well. 
A character-by-character search is more obtuse and any CPU cycle savings 
are pretty marginal. Also:


char const has_inc[] = "__has_include"; int const has_inc_len = 
sizeof(has_inc) - 1;


It makes what's going on more plain by eliminating a magic number (13).


+if (r >= base + match[0].rm_eo)
+  {
+base = r;
+goto again;
+  }
+  }
+
base += match[0].rm_eo;
  
/* One more test: if on the same line we have the same string

diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 3a4cfe06542..31389396af6 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -3151,7 +3151,8 @@ fix = {
  c_fix = machine_name;
  
  test_text = "/* MACH_DIFF: */\n"

-"#if defined( i386 ) || defined( sparc ) || defined( vax )"
+"#if defined( i386 ) || defined( sparc ) || defined( vax ) || "
+"defined( linux ) || __has_include (  ) || defined ( linux )"
No need for a redundant "defined(linux)" test. If you want to test 
superfluous spaces around the parentheses, just do it for one of the 
machine types.

  "\n/* no uniform test, so be careful  :-) */";
  };
  
diff --git a/fixincludes/tests/base/testing.h b/fixincludes/tests/base/testing.h

index cf95321fb86..00e8dde003e 100644
--- a/fixincludes/tests/base/testing.h
+++ b/fixincludes/tests/base/testing.h
@@ -64,7 +64,7 @@ BSD43__IOWR('T', 1) /* Some are multi-line */
  
  #if defined( MACHINE_NAME_CHECK )

  /* MACH_DIFF: */
-#if defined( i386 ) || defined( sparc ) || defined( vax )
+#if defined( i386 ) || defined( sparc ) || defined( vax ) || defined( linux ) || 
__has_include (  ) || defined ( linux )
  /* no uniform test, so be careful  :-) */
  #endif  /* MACHINE_NAME_CHECK */


Thanks for working on this.

Regards, Bruce

Re: [PATCH v3] fixinc: don't "fix" machine names in __has_include(...) [PR91085]

[Bruce Korb via Gcc-patches]

On 6/28/21 10:26 PM, Xi Ruoyao wrote:

v3:
   use memmem/memchr instead of trivial loops
   split most of the logic into a static function
   avoid hardcoded magic number
   adjust test

Looks good to me. :)