Do not reopen temporary files
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-10-27 Tom Tromey <tom@tromey.com> * 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.
This commit is contained in:
parent
b3279b601e
commit
36033ef57c
|
@ -1,3 +1,11 @@
|
||||||
|
2018-10-27 Tom Tromey <tom@tromey.com>
|
||||||
|
|
||||||
|
* 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.
|
||||||
|
|
||||||
2018-10-27 Tom Tromey <tom@tromey.com>
|
2018-10-27 Tom Tromey <tom@tromey.com>
|
||||||
|
|
||||||
* unittests/scoped_mmap-selftests.c (test_normal): Use
|
* unittests/scoped_mmap-selftests.c (test_normal): Use
|
||||||
|
|
|
@ -21,6 +21,7 @@
|
||||||
#define SCOPED_FD_H
|
#define SCOPED_FD_H
|
||||||
|
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
#include "filestuff.h"
|
||||||
|
|
||||||
/* A smart-pointer-like class to automatically close a file descriptor. */
|
/* A smart-pointer-like class to automatically close a file descriptor. */
|
||||||
|
|
||||||
|
@ -43,6 +44,18 @@ public:
|
||||||
return fd;
|
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
|
int get () const noexcept
|
||||||
{
|
{
|
||||||
return m_fd;
|
return m_fd;
|
||||||
|
|
|
@ -1566,23 +1566,21 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
|
||||||
? INDEX5_SUFFIX : INDEX4_SUFFIX));
|
? INDEX5_SUFFIX : INDEX4_SUFFIX));
|
||||||
gdb::char_vector filename_temp = make_temp_filename (filename);
|
gdb::char_vector filename_temp = make_temp_filename (filename);
|
||||||
|
|
||||||
gdb::optional<scoped_fd> out_file_fd
|
/* Order matters here; we want FILE to be closed before
|
||||||
(gdb::in_place, gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY));
|
FILENAME_TEMP is unlinked, because on MS-Windows one cannot
|
||||||
if (out_file_fd->get () == -1)
|
delete a file that is still open. So, we wrap the unlinker in an
|
||||||
|
optional and emplace it once we know the file name. */
|
||||||
|
gdb::optional<gdb::unlinker> unlink_file;
|
||||||
|
scoped_fd out_file_fd (gdb_mkostemp_cloexec (filename_temp.data (),
|
||||||
|
O_BINARY));
|
||||||
|
if (out_file_fd.get () == -1)
|
||||||
perror_with_name (("mkstemp"));
|
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)
|
if (out_file == nullptr)
|
||||||
error (_("Can't open `%s' for writing"), filename_temp.data ());
|
error (_("Can't open `%s' for writing"), filename_temp.data ());
|
||||||
|
|
||||||
/* Order matters here; we want FILE to be closed before FILENAME_TEMP is
|
unlink_file.emplace (filename_temp.data ());
|
||||||
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);
|
|
||||||
|
|
||||||
if (index_kind == dw_index_kind::DEBUG_NAMES)
|
if (index_kind == dw_index_kind::DEBUG_NAMES)
|
||||||
{
|
{
|
||||||
|
@ -1590,45 +1588,45 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
|
||||||
+ basename + DEBUG_STR_SUFFIX);
|
+ basename + DEBUG_STR_SUFFIX);
|
||||||
gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
|
gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
|
||||||
|
|
||||||
gdb::optional<scoped_fd> out_file_str_fd
|
/* As above, arrange to unlink the file only after the file
|
||||||
(gdb::in_place, gdb_mkostemp_cloexec (filename_str_temp.data (),
|
descriptor has been closed. */
|
||||||
O_BINARY));
|
gdb::optional<gdb::unlinker> unlink_file_str;
|
||||||
if (out_file_str_fd->get () == -1)
|
scoped_fd out_file_str_fd
|
||||||
|
(gdb_mkostemp_cloexec (filename_str_temp.data (), O_BINARY));
|
||||||
|
if (out_file_str_fd.get () == -1)
|
||||||
perror_with_name (("mkstemp"));
|
perror_with_name (("mkstemp"));
|
||||||
|
|
||||||
FILE *out_file_str
|
gdb_file_up out_file_str = out_file_str_fd.to_file ("wb");
|
||||||
= gdb_fopen_cloexec (filename_str_temp.data (), "wb").release ();
|
|
||||||
if (out_file_str == nullptr)
|
if (out_file_str == nullptr)
|
||||||
error (_("Can't open `%s' for writing"), filename_str_temp.data ());
|
error (_("Can't open `%s' for writing"), filename_str_temp.data ());
|
||||||
|
|
||||||
out_file_str_fd.reset ();
|
unlink_file_str.emplace (filename_str_temp.data ());
|
||||||
gdb::unlinker unlink_file_str (filename_str_temp.data ());
|
|
||||||
gdb_file_up close_out_file_str (out_file_str);
|
|
||||||
|
|
||||||
const size_t total_len
|
const size_t total_len
|
||||||
= write_debug_names (dwarf2_per_objfile, out_file, out_file_str);
|
= write_debug_names (dwarf2_per_objfile, out_file.get (),
|
||||||
assert_file_size (out_file, filename_temp.data (), total_len);
|
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. */
|
/* 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 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)
|
if (rename (filename_str_temp.data (), filename_str.c_str ()) != 0)
|
||||||
perror_with_name (("rename"));
|
perror_with_name (("rename"));
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
const size_t total_len
|
const size_t total_len
|
||||||
= write_gdbindex (dwarf2_per_objfile, out_file);
|
= write_gdbindex (dwarf2_per_objfile, out_file.get ());
|
||||||
assert_file_size (out_file, filename_temp.data (), total_len);
|
assert_file_size (out_file.get (), filename_temp.data (), total_len);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* We want to keep the file. */
|
/* We want to keep the file. */
|
||||||
unlink_file.keep ();
|
unlink_file->keep ();
|
||||||
|
|
||||||
/* Close and move the file in place. */
|
/* Close and move the file in place. */
|
||||||
close_out_file.reset ();
|
out_file.reset ();
|
||||||
if (rename (filename_temp.data (), filename.c_str ()) != 0)
|
if (rename (filename_temp.data (), filename.c_str ()) != 0)
|
||||||
perror_with_name (("rename"));
|
perror_with_name (("rename"));
|
||||||
}
|
}
|
||||||
|
|
|
@ -65,12 +65,29 @@ test_release ()
|
||||||
SELF_CHECK (close (fd) == 0 || errno != EBADF);
|
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_mkostemp_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. */
|
/* Run selftests. */
|
||||||
static void
|
static void
|
||||||
run_tests ()
|
run_tests ()
|
||||||
{
|
{
|
||||||
test_destroy ();
|
test_destroy ();
|
||||||
test_release ();
|
test_release ();
|
||||||
|
test_to_file ();
|
||||||
}
|
}
|
||||||
|
|
||||||
} /* namespace scoped_fd */
|
} /* namespace scoped_fd */
|
||||||
|
|
Loading…
Reference in New Issue