From patchwork Sun Nov 10 20:43:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janne Blomqvist X-Patchwork-Id: 290100 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6FE362C00B8 for ; Mon, 11 Nov 2013 07:43:56 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=W6w1Eu5uiEqfthBwga z/5mv7IPte2HFQOdaLnafEyGt4UyRCrj0r3zHXjcSpiONMPeSEl3VgIN9vliVRB/ MTnhqDGaiByFP2alWLRyd01i0m9JDtDDo6VAhmOwu2mtGhvqNC4QxdCwZl4ND/F6 5gTpWDptgPT7blOIERIkRUF3Y= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=NlCuZHZJBm2c1U0CMkXBuw6W SyA=; b=p4eFY2rDSEU2cMJ+vZpuWd8oMc3dH+ecDI6hKcHcDEIGz4RbcXpx5X6R po5Y4eq3gpfQektQGmecuvoWsyMW3PdYTMHhrFfbKWkoSZE2+KTwFVDQvNF8bM2F ajDi7xBbkPpSAmRFHMl+a/on+uwSyHRmD5r7DTI0fO8IJhBs0bw= Received: (qmail 19141 invoked by alias); 10 Nov 2013 20:43:35 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 19121 invoked by uid 89); 10 Nov 2013 20:43:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, RDNS_NONE, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-ob0-f176.google.com Received: from Unknown (HELO mail-ob0-f176.google.com) (209.85.214.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sun, 10 Nov 2013 20:43:33 +0000 Received: by mail-ob0-f176.google.com with SMTP id wp4so2215821obc.7 for ; Sun, 10 Nov 2013 12:43:25 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.182.230.135 with SMTP id sy7mr16103711obc.24.1384116205524; Sun, 10 Nov 2013 12:43:25 -0800 (PST) Received: by 10.182.146.15 with HTTP; Sun, 10 Nov 2013 12:43:25 -0800 (PST) In-Reply-To: <527FBF60.20702@net-b.de> References: <527FBF60.20702@net-b.de> Date: Sun, 10 Nov 2013 22:43:25 +0200 Message-ID: Subject: Re: [Patch, libgfortran] Set close-on-exec flag when opening files From: Janne Blomqvist To: Tobias Burnus Cc: Fortran List , GCC Patches On Sun, Nov 10, 2013 at 7:16 PM, Tobias Burnus wrote: > Janne Blomqvist wrote: >> >> the attached patch sets the close-on-exec flag when opening files, as >> is usually considered good practice these days. See e.g. >> http://www.python.org/dev/peps/pep-0446/ and links therein for more >> information. > > >> + int flags = O_RDWR|O_CREAT|O_EXCL; > > I'd add spaces around "|". > > > Otherwise, it looks good to me. Thanks for the patch and sorry for the slow > review. Thanks for the review, committed as r204654. The committed patch differs from the one sent for review in the following aspects: - Fixed the issue you mentioned above - Fixed another "spaces around |" issue - In set_close_on_exec(), call fcntl only if fd >= 0; this prevents clobbering errno if something went wrong earlier - Add __attribute__((unused)) to set_close_on_exec, preventing a warning on systems having both O_CLOEXEC and mkostemp() (such as recent Linux) Committed patch attached. I'll add a note to the wiki as well, to be added to the release notes at some later point by someone. diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 4609eba..6417373 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -280,7 +280,7 @@ else strcasestr getrlimit gettimeofday stat fstat lstat getpwuid vsnprintf dup \ getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \ readlink getgid getpid getppid getuid geteuid umask getegid \ - secure_getenv __secure_getenv) + secure_getenv __secure_getenv mkostemp) fi # Check strerror_r, cannot be above as versions with two and three arguments exist diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c index dd2715b..8a84ae4 100644 --- a/libgfortran/io/unix.c +++ b/libgfortran/io/unix.c @@ -1070,6 +1070,20 @@ unpack_filename (char *cstring, const char *fstring, int len) } +/* Set the close-on-exec flag for an existing fd, if the system + supports such. */ + +static void __attribute__ ((unused)) +set_close_on_exec (int fd __attribute__ ((unused))) +{ + /* Mingw does not define F_SETFD. */ +#if defined(F_SETFD) && defined(FD_CLOEXEC) + if (fd >= 0) + fcntl(fd, F_SETFD, FD_CLOEXEC); +#endif +} + + /* Helper function for tempfile(). Tries to open a temporary file in the directory specified by tempdir. If successful, the file name is stored in fname and the descriptor returned. Returns -1 on @@ -1109,7 +1123,12 @@ tempfile_open (const char *tempdir, char **fname) mode_mask = umask (S_IXUSR | S_IRWXG | S_IRWXO); #endif +#if defined(HAVE_MKOSTEMP) && defined(O_CLOEXEC) + fd = mkostemp (template, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC); +#else fd = mkstemp (template); + set_close_on_exec (fd); +#endif #ifdef HAVE_UMASK (void) umask (mode_mask); @@ -1119,6 +1138,13 @@ tempfile_open (const char *tempdir, char **fname) fd = -1; int count = 0; size_t slashlen = strlen (slash); + int flags = O_RDWR | O_CREAT | O_EXCL; +#if defined(HAVE_CRLF) && defined(O_BINARY) + flags |= O_BINARY; +#endif +#ifdef O_CLOEXEC + flags |= O_CLOEXEC; +#endif do { snprintf (template, tempdirlen + 23, "%s%sgfortrantmpaaaXXXXXX", @@ -1142,14 +1168,12 @@ tempfile_open (const char *tempdir, char **fname) continue; } -#if defined(HAVE_CRLF) && defined(O_BINARY) - fd = open (template, O_RDWR | O_CREAT | O_EXCL | O_BINARY, - S_IRUSR | S_IWUSR); -#else - fd = open (template, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR); -#endif + fd = open (template, flags, S_IRUSR | S_IWUSR); } while (fd == -1 && errno == EEXIST); +#ifndef O_CLOEXEC + set_close_on_exec (fd); +#endif #endif /* HAVE_MKSTEMP */ *fname = template; @@ -1323,6 +1347,10 @@ regular_file (st_parameter_open *opp, unit_flags *flags) crflag |= O_BINARY; #endif +#ifdef O_CLOEXEC + crflag |= O_CLOEXEC; +#endif + mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; fd = open (path, rwflag | crflag, mode); if (flags->action != ACTION_UNSPECIFIED) @@ -1386,6 +1414,9 @@ open_external (st_parameter_open *opp, unit_flags *flags) /* regular_file resets flags->action if it is ACTION_UNSPECIFIED and * if it succeeds */ fd = regular_file (opp, flags); +#ifndef O_CLOEXEC + set_close_on_exec (fd); +#endif } if (fd < 0)