This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix file descriptor existence of MinGW.


On 8/6/19 6:18 PM, Martin Sebor wrote:
> On 8/6/19 9:55 AM, Martin Liška wrote:
>> On 8/6/19 5:35 PM, Martin Sebor wrote:
>>> On 8/6/19 6:04 AM, Martin Liška wrote:
>>>> Hi.
>>>>
>>>> The patch is about proper checking of file descriptors on Windows.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Is there a way to share the definition of the new function so
>>> it doesn't have to be duplicated?
>>
>> I would like to, but I'm wondering which header and source file
>> should I use for it?
> 
> I'm not sure.  It's a general purpose function that could go
> in some utility library like libiberty but I don't know what
> the process for enhancing it is to tell if that's appropriate.

Yep, I like the suggested approach. I've done that in the updated
patch.

> 
>>> Other than that, I'm wondering if the guard for F_GETFD is
>>> necessary and when (the original code didn't guard its use).
>>
>> Because the value is not defined on MinGW targets where _get_osfhandle
>> call must be used ;)
> 
> I was unclear -- sorry.  What I meant was: since there is
> a check for MinGW, is the check for F_GETFD necessary, or
> would this be sufficient and preferable?
> 
>   #if defined(_WIN32)
>     return _get_osfhandle (fd) != -1;
>   else
>     return fcntl (fd, F_GETFD) >= 0;
>   #endif
> 
> That way it's clear that only two possibilities are expected.

Ah yes, included.

Martin

> 
> Martin
> 
>>
>> Martin
>>
>>>
>>> Martin
>>>
>>>
>>>>
>>>> @Pekka: Can you please test it on Windows?
>>>>
>>>> Ready to be installed?
>>>> Thanks,
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2019-08-06  Martin Liska  <mliska@suse.cz>
>>>>
>>>>      PR bootstrap/91352
>>>>      * gcc.c (fd_exists): New.
>>>>      (driver::detect_jobserver): Use fd_exists.
>>>>      * lto-wrapper.c (fd_exists): New.
>>>>      (jobserver_active_p): Use fd_exists.
>>>> ---
>>>>    gcc/gcc.c         | 19 +++++++++++++++++--
>>>>    gcc/lto-wrapper.c | 19 +++++++++++++++++--
>>>>    2 files changed, 34 insertions(+), 4 deletions(-)
>>>>
>>>>
>>>
>>
> 

>From e76fdfabf88305b26eb53690c251d016e6c5e455 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 6 Aug 2019 13:04:40 +0200
Subject: [PATCH] Fix file descriptor existence of MinGW.

gcc/ChangeLog:

2019-08-07  Martin Liska  <mliska@suse.cz>

	PR bootstrap/91352
	* gcc.c (driver::detect_jobserver): Use fd_exists.
	* lto-wrapper.c (jobserver_active_p): Likewise.

include/ChangeLog:

2019-08-07  Martin Liska  <mliska@suse.cz>

	PR bootstrap/91352
	* libiberty.h (fd_exists): New function.

libiberty/ChangeLog:

2019-08-07  Martin Liska  <mliska@suse.cz>

	PR bootstrap/91352
	* lrealpath.c (fd_exists): New function.
---
 gcc/gcc.c             |  4 ++--
 gcc/lto-wrapper.c     |  4 ++--
 include/libiberty.h   |  4 ++++
 libiberty/lrealpath.c | 14 ++++++++++++++
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 18a07426290..e86b35453c2 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -8380,8 +8380,8 @@ driver::detect_jobserver () const
 	    = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2
 	       && rfd > 0
 	       && wfd > 0
-	       && fcntl (rfd, F_GETFD) >= 0
-	       && fcntl (wfd, F_GETFD) >= 0);
+	       && fd_exists (rfd)
+	       && fd_exists (wfd));
 
 	  /* Drop the jobserver if it's not working now.  */
 	  if (!jobserver)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 3414adedd26..a3b0130b7cb 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1237,8 +1237,8 @@ jobserver_active_p (void)
   return (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2
 	  && rfd > 0
 	  && wfd > 0
-	  && fcntl (rfd, F_GETFD) >= 0
-	  && fcntl (wfd, F_GETFD) >= 0);
+	  && fd_exists (rfd)
+	  && fd_exists (wfd));
 }
 
 /* Execute gcc. ARGC is the number of arguments. ARGV contains the arguments. */
diff --git a/include/libiberty.h b/include/libiberty.h
index 635519e088a..254c00bcc6b 100644
--- a/include/libiberty.h
+++ b/include/libiberty.h
@@ -137,6 +137,10 @@ extern const char *unix_lbasename (const char *) ATTRIBUTE_RETURNS_NONNULL ATTRI
 
 extern char *lrealpath (const char *);
 
+/* Return true when FD file descriptor exists.  */
+
+extern int fd_exists (int fd);
+
 /* Concatenate an arbitrary number of strings.  You must pass NULL as
    the last argument of this function, to terminate the list of
    strings.  Allocates memory using xmalloc.  */
diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 7f66dc2b1bd..4fdb522e3f2 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -49,6 +49,7 @@ components will be simplified.  The returned value will be allocated using
 #ifdef HAVE_STRING_H
 #include <string.h>
 #endif
+#include <fcntl.h>
 
 /* On GNU libc systems the declaration is only visible with _GNU_SOURCE.  */
 #if defined(HAVE_CANONICALIZE_FILE_NAME) \
@@ -155,3 +156,16 @@ lrealpath (const char *filename)
   /* This system is a lost cause, just duplicate the filename.  */
   return strdup (filename);
 }
+
+/* Return true when FD file descriptor exists.  */
+
+int
+fd_exists (int fd)
+{
+#if defined(_WIN32)
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  return h != (HANDLE) -1;
+#else
+  return fcntl (fd, F_GETFD) >= 0;
+#endif
+}
-- 
2.22.0


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]