From patchwork Wed Sep 26 11:11:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 29544 Received: (qmail 93804 invoked by alias); 26 Sep 2018 11:12:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 93710 invoked by uid 89); 26 Sep 2018 11:12:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=possession, run_tests X-HELO: gateway22.websitewelcome.com Received: from gateway22.websitewelcome.com (HELO gateway22.websitewelcome.com) (192.185.47.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Sep 2018 11:12:36 +0000 Received: from cm15.websitewelcome.com (cm15.websitewelcome.com [100.42.49.9]) by gateway22.websitewelcome.com (Postfix) with ESMTP id 3A88F1B3DE for ; Wed, 26 Sep 2018 06:12:35 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id 57jIgH5rT8YaU57jnglBXu; Wed, 26 Sep 2018 06:12:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=wB0gySwtz4Dnrg2PgdKKRwNHZkMRjj3ZmVoQFS8rN5M=; b=x8A8dhwXLQVakV622wEuzzLErx 5pkL5SuUuDJkdIRFXazkyLunx1jGKhvX49eO+4sJv+vEiCgWNbeaZqtWlYsSv95MN42APIYWLrQTu MsUf+D1yiREdCH+IaN6KBe03h; Received: from 97-122-190-66.hlrn.qwest.net ([97.122.190.66]:44312 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1g57jI-0014pw-Bw; Wed, 26 Sep 2018 06:11:40 -0500 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFC 5/6] Do not reopen temporary files Date: Wed, 26 Sep 2018 05:11:29 -0600 Message-Id: <20180926111130.18956-6-tom@tromey.com> In-Reply-To: <20180926111130.18956-1-tom@tromey.com> References: <20180926111130.18956-1-tom@tromey.com> The current callers of mkostemp close the file descriptor and then re-open it with fopen. It seemed better to me to continue to use the already-opened file descriptor, so this patch rearranges the code a little in order to do so. It takes care to ensure that the files are only unlinked after the file descriptor in question is closed, as before. gdb/ChangeLog 2018-09-26 Tom Tromey * unittests/scoped_fd-selftests.c (test_to_file): New function. (run_tests): Call test_to_file. * dwarf-index-write.c (write_psymtabs_to_index): Do not reopen temporary files. * common/scoped_fd.h (scoped_fd::to_file): New method. --- gdb/ChangeLog | 8 +++++ gdb/common/scoped_fd.h | 13 +++++++ gdb/dwarf-index-write.c | 55 ++++++++++++++--------------- gdb/unittests/scoped_fd-selftests.c | 17 +++++++++ 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h index a6a8ab9a38..f37776ae21 100644 --- a/gdb/common/scoped_fd.h +++ b/gdb/common/scoped_fd.h @@ -25,6 +25,7 @@ #ifdef HAVE_UNISTD_H #include +#include "filestuff.h" /* A smart-pointer-like class to automatically close a file descriptor. */ @@ -47,6 +48,18 @@ public: return fd; } + /* Like release, but return a gdb_file_up that owns the file + descriptor. On success, this scoped_fd will be released. On + failure, return NULL and leave this scoped_fd in possession of + the fd. */ + gdb_file_up to_file (const char *mode) noexcept + { + gdb_file_up result (fdopen (m_fd, mode)); + if (result != nullptr) + m_fd = -1; + return result; + } + int get () const noexcept { return m_fd; diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c index 7049e8968a..341e16bfc5 100644 --- a/gdb/dwarf-index-write.c +++ b/gdb/dwarf-index-write.c @@ -1565,23 +1565,21 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile, ? INDEX5_SUFFIX : INDEX4_SUFFIX)); gdb::char_vector filename_temp = make_temp_filename (filename); - gdb::optional out_file_fd - (gdb::in_place, gdb_mkstemp_cloexec (filename_temp.data ())); - if (out_file_fd->get () == -1) + /* Order matters here; we want FILE to be closed before FILENAME_TEMP is + unlinked, because on MS-Windows one cannot delete a file that is + still open. (Don't call anything here that might throw until + file_closer is created.) So, we wrap the unlinker in an optional + and emplace it once we know the file name. */ + gdb::optional unlink_file; + scoped_fd out_file_fd (gdb_mkstemp_cloexec (filename_temp.data ())); + if (out_file_fd.get () == -1) perror_with_name (("mkstemp")); - FILE *out_file = gdb_fopen_cloexec (filename_temp.data (), "wb").release (); + gdb_file_up out_file = out_file_fd.to_file ("wb"); if (out_file == nullptr) error (_("Can't open `%s' for writing"), filename_temp.data ()); - /* Order matters here; we want FILE to be closed before FILENAME_TEMP is - unlinked, because on MS-Windows one cannot delete a file that is - still open. (Don't call anything here that might throw until - file_closer is created.) We don't need OUT_FILE_FD anymore, so we might - as well close it now. */ - out_file_fd.reset (); - gdb::unlinker unlink_file (filename_temp.data ()); - gdb_file_up close_out_file (out_file); + unlink_file.emplace (filename_temp.data ()); if (index_kind == dw_index_kind::DEBUG_NAMES) { @@ -1589,44 +1587,45 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile, + basename + DEBUG_STR_SUFFIX); gdb::char_vector filename_str_temp = make_temp_filename (filename_str); - gdb::optional out_file_str_fd - (gdb::in_place, gdb_mkstemp_cloexec (filename_str_temp.data ())); - if (out_file_str_fd->get () == -1) + /* As above, arrange to unlink the file only after the file + descriptor has been closed. */ + gdb::optional unlink_file_str; + scoped_fd out_file_str_fd + (gdb_mkstemp_cloexec (filename_str_temp.data ())); + if (out_file_str_fd.get () == -1) perror_with_name (("mkstemp")); - FILE *out_file_str - = gdb_fopen_cloexec (filename_str_temp.data (), "wb").release (); + gdb_file_up out_file_str = out_file_str_fd.to_file ("wb"); if (out_file_str == nullptr) error (_("Can't open `%s' for writing"), filename_str_temp.data ()); - out_file_str_fd.reset (); - gdb::unlinker unlink_file_str (filename_str_temp.data ()); - gdb_file_up close_out_file_str (out_file_str); + unlink_file_str.emplace (filename_str_temp.data ()); const size_t total_len - = write_debug_names (dwarf2_per_objfile, out_file, out_file_str); - assert_file_size (out_file, filename_temp.data (), total_len); + = write_debug_names (dwarf2_per_objfile, out_file.get (), + out_file_str.get ()); + assert_file_size (out_file.get (), filename_temp.data (), total_len); /* We want to keep the file .debug_str file too. */ - unlink_file_str.keep (); + unlink_file_str->keep (); /* Close and move the str file in place. */ - close_out_file_str.reset (); + out_file_str.reset (); if (rename (filename_str_temp.data (), filename_str.c_str ()) != 0) perror_with_name (("rename")); } else { const size_t total_len - = write_gdbindex (dwarf2_per_objfile, out_file); - assert_file_size (out_file, filename_temp.data (), total_len); + = write_gdbindex (dwarf2_per_objfile, out_file.get ()); + assert_file_size (out_file.get (), filename_temp.data (), total_len); } /* We want to keep the file. */ - unlink_file.keep (); + unlink_file->keep (); /* Close and move the file in place. */ - close_out_file.reset (); + out_file.reset (); if (rename (filename_temp.data (), filename.c_str ()) != 0) perror_with_name (("rename")); } diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c index 4320b540cd..4690ae79f5 100644 --- a/gdb/unittests/scoped_fd-selftests.c +++ b/gdb/unittests/scoped_fd-selftests.c @@ -68,12 +68,29 @@ test_release () SELF_CHECK (close (fd) == 0 || errno != EBADF); } +/* Test that the file descriptor can be converted to a FILE *. */ +static void +test_to_file () +{ + char filename[] = "scoped_fd-selftest-XXXXXX"; + + ::scoped_fd sfd (gdb_mkstemp_cloexec (filename)); + SELF_CHECK (sfd.get () >= 0); + + unlink (filename); + + gdb_file_up file = sfd.to_file ("rw"); + SELF_CHECK (file != nullptr); + SELF_CHECK (sfd.get () == -1); +} + /* Run selftests. */ static void run_tests () { test_destroy (); test_release (); + test_to_file (); } } /* namespace scoped_fd */