Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Eli Zaretskii <eliz@gnu.org>, Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org, vleschuk@accesssoftek.com
Subject: [pushed] .gdb_index writer: close the file before unlinking it (Re: [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer.)
Date: Mon, 19 Jun 2017 11:50:00 -0000	[thread overview]
Message-ID: <b9339642-38f2-ec56-2fb1-c2c03983fa34@redhat.com> (raw)
In-Reply-To: <83poe1xrdr.fsf@gnu.org>


On 06/18/2017 04:11 PM, Eli Zaretskii wrote:
>> Date: Sun, 18 Jun 2017 16:25:30 +0200
>> From: Jan Kratochvil <jan.kratochvil@redhat.com>
>> Cc: gdb-patches@sourceware.org, Victor Leschuk <vleschuk@accesssoftek.com>
>>
>> On Mon, 12 Jun 2017 18:08:07 +0200, Pedro Alves wrote:
>>> +  file_closer close_out_file (out_file);
>>> +  gdb::unlinker unlink_file (filename.c_str ());
>>
>> I heard on MS-Windows one cannot delete a file which is still open.
> 
> Depends on how it was open, but yeah, in general you are right.  (Most
> programs, especially those which were ported from Posix systems, open
> files in a way that indeed precludes their deletion, because that's
> what the MS implementation of 'open' does.)

Yeah, I forgot/missed that here.

I've pushed this in to fix it.

From 16b7a7199881fa26fc863279bbf08741e5674b5d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 19 Jun 2017 12:46:47 +0100
Subject: [PATCH] .gdb_index writer: close the file before unlinking it

We should close the file before unlinking because on MS-Windows one
cannot delete a file that is still open.

I considered making 'gdb::unlinker::unlinker(const char *)'
'noexcept(true)' and then adding
  static_assert (noexcept (gdb::unlinker (filename.c_str ())), "");

but that doesn't really work because gdb::unlinker has a gdb_assert,
which can throw a QUIT if/when the assertion fails.  'noexcept(true)'
would cause GDB to abruptly terminate if/when the assertion fails.

gdb/ChangeLog:
2017-06-19  Pedro Alves  <palves@redhat.com>

	* dwarf2read.c (write_psymtabs_to_index): Construct file_closer
	after gdb::unlinker.
---
 gdb/ChangeLog    | 5 +++++
 gdb/dwarf2read.c | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index aaf4b89..c7cf410 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-06-19  Pedro Alves  <palves@redhat.com>
+
+	* dwarf2read.c (write_psymtabs_to_index): Construct file_closer
+	after gdb::unlinker.
+
 2017-06-19  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	* mi/mi-cm-env.c (_initialize_mi_cmd_env): Use getenv instead of
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index abe14b2..2369d4b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -23776,8 +23776,12 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
   if (!out_file)
     error (_("Can't open `%s' for writing"), filename.c_str ());
 
-  file_closer close_out_file (out_file);
+  /* Order matters here; we want FILE to be closed before FILENAME 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.)  */
   gdb::unlinker unlink_file (filename.c_str ());
+  file_closer close_out_file (out_file);
 
   mapped_symtab symtab;
   data_buf cu_list;
-- 
2.5.5


  reply	other threads:[~2017-06-19 11:50 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-26 18:25 [PATCH 0/6] DWARF-5: .debug_names index Jan Kratochvil
2017-05-26 18:25 ` [PATCH 2/6] cc-with-tweaks.sh: Use gdb-add-index.sh Jan Kratochvil
2017-05-26 18:25 ` [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Jan Kratochvil
2017-06-12 16:08   ` [pushed] " Pedro Alves
2017-06-12 16:14     ` [PATCH 3/6] Code cleanup: dwarf2read.c: Add data_buf::append_uint Pedro Alves
2017-06-12 16:14     ` [PATCH 1/6] Code cleanup: dwarf2read.c:uniquify_cu_indices: Use std::unique Pedro Alves
2017-06-12 16:14     ` [PATCH 4/6] .gdb_index prod perf regression: find before insert in unordered_map Pedro Alves
2017-06-12 16:14     ` [PATCH 6/6] .gdb_index prod perf regression: mapped_symtab now vector of values Pedro Alves
2017-06-12 16:14     ` [PATCH 2/6] Code cleanup: dwarf2read.c: Eliminate ::file_write Pedro Alves
2017-06-17 17:35       ` Jan Kratochvil
2017-06-19  9:26         ` Pedro Alves
2017-06-18 18:36       ` Regression: " Jan Kratochvil
2017-06-19  9:27         ` Pedro Alves
2017-06-19  9:39           ` Jan Kratochvil
2017-06-19  9:47             ` Pedro Alves
2017-06-19 10:03               ` Jan Kratochvil
2017-06-19 10:35                 ` Pedro Alves
2017-06-19 12:06                 ` Yao Qi
2017-06-19 12:26                   ` Jan Kratochvil
2017-06-12 16:18     ` [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Pedro Alves
2017-06-12 16:19     ` [PATCH 5/6] .gdb_index prod perf regression: Estimate size of psyms_seen Pedro Alves
2017-06-18 14:25     ` [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Jan Kratochvil
2017-06-18 15:12       ` Eli Zaretskii
2017-06-19 11:50         ` Pedro Alves [this message]
2017-06-18 16:50     ` Jan Kratochvil
2017-05-26 18:26 ` [PATCH 5/6] Refactor: Move some generic code out of .gdb_index code Jan Kratochvil
2017-05-26 18:26 ` [PATCH 6/6] DWARF-5: .debug_names index consumer Jan Kratochvil
2017-05-26 18:26 ` [PATCH 3/6] DWARF-5: .debug_names index producer Jan Kratochvil
2017-06-09  5:58   ` [PATCH 3.1/6] " Jan Kratochvil
2017-05-26 18:26 ` [PATCH 4/6] Code cleanup: dwarf2_initialize_objfile return value Jan Kratochvil
2017-06-18 19:37 ` obsolete: [PATCH 0/6] DWARF-5: .debug_names index Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9339642-38f2-ec56-2fb1-c2c03983fa34@redhat.com \
    --to=palves@redhat.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=vleschuk@accesssoftek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox