* [PATCH v2 4/6] Use mkostemp, not mkstemp
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
` (3 preceding siblings ...)
2018-10-03 21:02 ` [PATCH v2 5/6] Do not reopen temporary files Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
2018-10-03 21:02 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed that gdb could leak file descriptors coming from mkstemp.
This patch fixes the problem by importing the gnulib mkostemp instead,
and then changing gdb to pass O_CLOEXEC.
A small gnulib patch was needed. This has already been accepted
upstream.
gdb/ChangeLog
2018-10-03 Tom Tromey <tom@tromey.com>
* unittests/scoped_mmap-selftests.c (test_normal): Use
gdb_mkostemp_cloexec.
* unittests/scoped_fd-selftests.c (test_destroy, test_release):
Use gdb_mkostemp_cloexec.
* gnulib/aclocal-m4-deps.mk, gnulib/aclocal.m4,
gnulib/config.in, gnulib/configure,
gnulib/import/Makefile.am, gnulib/import/Makefile.in,
gnulib/import/m4/gnulib-cache.m4,
gnulib/import/m4/gnulib-comp.m4: Update.
* gnulib/import/m4/mkostemp.m4: New file.
* gnulib/import/m4/mkstemp.m4: Remove.
* gnulib/import/mkostemp.c: New file.
* gnulib/import/mkstemp.m4: Remove.
* gnulib/update-gnulib.sh (IMPORTED_GNULIB_MODULES): Remove
mkstemp, add mkostemp. Apply new patch.
* gnulib/import/stdlib.in.h: Apply patch.
* gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch:
New file.
* dwarf-index-write.c (write_psymtabs_to_index): Use
gdb_mkostemp_cloexec.
* common/filestuff.h (gdb_mkostemp_cloexec): New function.
---
gdb/ChangeLog | 24 +++++
gdb/common/filestuff.h | 11 +++
gdb/dwarf-index-write.c | 5 +-
gdb/gnulib/aclocal-m4-deps.mk | 2 +-
gdb/gnulib/aclocal.m4 | 2 +-
gdb/gnulib/config.in | 12 ++-
gdb/gnulib/configure | 95 +++----------------
gdb/gnulib/import/Makefile.am | 10 +-
gdb/gnulib/import/Makefile.in | 18 ++--
gdb/gnulib/import/m4/gnulib-cache.m4 | 4 +-
gdb/gnulib/import/m4/gnulib-comp.m4 | 17 ++--
gdb/gnulib/import/m4/mkostemp.m4 | 23 +++++
gdb/gnulib/import/m4/mkstemp.m4 | 82 ----------------
gdb/gnulib/import/{mkstemp.c => mkostemp.c} | 12 +--
gdb/gnulib/import/stdlib.in.h | 3 +-
...ps-Fix-compilation-error-in-C-mode-o.patch | 38 ++++++++
gdb/gnulib/update-gnulib.sh | 3 +-
gdb/unittests/scoped_fd-selftests.c | 5 +-
gdb/unittests/scoped_mmap-selftests.c | 3 +-
19 files changed, 162 insertions(+), 207 deletions(-)
create mode 100644 gdb/gnulib/import/m4/mkostemp.m4
delete mode 100644 gdb/gnulib/import/m4/mkstemp.m4
rename gdb/gnulib/import/{mkstemp.c => mkostemp.c} (79%)
create mode 100644 gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index ecfc18d600..9a12f83c27 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -20,6 +20,7 @@
#define FILESTUFF_H
#include <dirent.h>
+#include <fcntl.h>
/* Note all the file descriptors which are open when this is called.
These file descriptors will not be closed by close_most_fds. */
@@ -48,6 +49,16 @@ extern void close_most_fds (void);
extern int gdb_open_cloexec (const char *filename, int flags,
/* mode_t */ unsigned long mode);
+/* Like mkstemp, but ensures that the file descriptor is
+ close-on-exec. */
+
+static inline int
+gdb_mkostemp_cloexec (char *name_template, int flags = 0)
+{
+ /* gnulib provides a mkostemp replacement if needed. */
+ return mkostemp (name_template, flags | O_CLOEXEC);
+}
+
/* Convenience wrapper for the above, which takes the filename as an
std::string. */
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index dc8660e734..3032798426 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -1566,7 +1566,7 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
gdb::char_vector filename_temp = make_temp_filename (filename);
gdb::optional<scoped_fd> out_file_fd
- (gdb::in_place, mkstemp (filename_temp.data ()));
+ (gdb::in_place, gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY));
if (out_file_fd->get () == -1)
perror_with_name (("mkstemp"));
@@ -1590,7 +1590,8 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
gdb::optional<scoped_fd> out_file_str_fd
- (gdb::in_place, mkstemp (filename_str_temp.data ()));
+ (gdb::in_place, gdb_mkostemp_cloexec (filename_str_temp.data (),
+ O_BINARY));
if (out_file_str_fd->get () == -1)
perror_with_name (("mkstemp"));
diff --git a/gdb/gnulib/aclocal-m4-deps.mk b/gdb/gnulib/aclocal-m4-deps.mk
index d866b6de89..5b2c6cc5ea 100644
--- a/gdb/gnulib/aclocal-m4-deps.mk
+++ b/gdb/gnulib/aclocal-m4-deps.mk
@@ -80,7 +80,7 @@ aclocal_m4_deps = \
import/m4/mempcpy.m4 \
import/m4/memrchr.m4 \
import/m4/mkdir.m4 \
- import/m4/mkstemp.m4 \
+ import/m4/mkostemp.m4 \
import/m4/mmap-anon.m4 \
import/m4/mode_t.m4 \
import/m4/msvc-inval.m4 \
diff --git a/gdb/gnulib/aclocal.m4 b/gdb/gnulib/aclocal.m4
index 740387ac34..861caf6692 100644
--- a/gdb/gnulib/aclocal.m4
+++ b/gdb/gnulib/aclocal.m4
@@ -1353,7 +1353,7 @@ m4_include([import/m4/memmem.m4])
m4_include([import/m4/mempcpy.m4])
m4_include([import/m4/memrchr.m4])
m4_include([import/m4/mkdir.m4])
-m4_include([import/m4/mkstemp.m4])
+m4_include([import/m4/mkostemp.m4])
m4_include([import/m4/mmap-anon.m4])
m4_include([import/m4/mode_t.m4])
m4_include([import/m4/msvc-inval.m4])
diff --git a/gdb/gnulib/config.in b/gdb/gnulib/config.in
index d32b192e0b..d23d208cb2 100644
--- a/gdb/gnulib/config.in
+++ b/gdb/gnulib/config.in
@@ -95,6 +95,10 @@
whether the gnulib module getcwd shall be considered present. */
#undef GNULIB_GETCWD
+/* Define to a C preprocessor expression that evaluates to 1 or 0, depending
+ whether the gnulib module mkostemp shall be considered present. */
+#undef GNULIB_MKOSTEMP
+
/* Define to a C preprocessor expression that evaluates to 1 or 0, depending
whether the gnulib module openat shall be considered present. */
#undef GNULIB_OPENAT
@@ -199,8 +203,8 @@
/* Define to 1 when the gnulib module memrchr should be tested. */
#undef GNULIB_TEST_MEMRCHR
-/* Define to 1 when the gnulib module mkstemp should be tested. */
-#undef GNULIB_TEST_MKSTEMP
+/* Define to 1 when the gnulib module mkostemp should be tested. */
+#undef GNULIB_TEST_MKOSTEMP
/* Define to 1 when the gnulib module open should be tested. */
#undef GNULIB_TEST_OPEN
@@ -544,8 +548,8 @@
when it succeeds. */
#undef HAVE_MINIMALLY_WORKING_GETCWD
-/* Define to 1 if you have the 'mkstemp' function. */
-#undef HAVE_MKSTEMP
+/* Define to 1 if you have the 'mkostemp' function. */
+#undef HAVE_MKOSTEMP
/* Define to 1 if you have the 'mprotect' function. */
#undef HAVE_MPROTECT
diff --git a/gdb/gnulib/configure b/gdb/gnulib/configure
index dc6c5b6657..5d7f8aa004 100644
--- a/gdb/gnulib/configure
+++ b/gdb/gnulib/configure
@@ -3525,7 +3525,7 @@ gl_func_list="$gl_func_list mbsinit"
gl_func_list="$gl_func_list mbrtowc"
gl_header_list="$gl_header_list sys/mman.h"
gl_func_list="$gl_func_list mprotect"
-gl_func_list="$gl_func_list mkstemp"
+gl_func_list="$gl_func_list mkostemp"
gl_func_list="$gl_func_list openat"
gl_func_list="$gl_func_list link"
gl_func_list="$gl_func_list secure_getenv"
@@ -5841,7 +5841,7 @@ fi
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
- # Code from module mkstemp:
+ # Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
# Code from module multiarch:
@@ -21893,116 +21893,51 @@ $as_echo "#define FUNC_MKDIR_DOT_BUG 1" >>confdefs.h
+
+
:
- if test $ac_cv_func_mkstemp = yes; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for working mkstemp" >&5
-$as_echo_n "checking for working mkstemp... " >&6; }
-if ${gl_cv_func_working_mkstemp+:} false; then :
- $as_echo_n "(cached) " >&6
-else
+ if test $ac_cv_func_mkostemp != yes; then
+ HAVE_MKOSTEMP=0
+ fi
- mkdir conftest.mkstemp
- if test "$cross_compiling" = yes; then :
- case "$host_os" in
- # Guess yes on glibc systems.
- *-gnu*) gl_cv_func_working_mkstemp="guessing yes" ;;
- # If we don't know, assume the worst.
- *) gl_cv_func_working_mkstemp="guessing no" ;;
- esac
+ if test $HAVE_MKOSTEMP = 0; then
-else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-$ac_includes_default
-int
-main ()
-{
-int result = 0;
- int i;
- off_t large = (off_t) 4294967295u;
- if (large < 0)
- large = 2147483647;
- umask (0);
- for (i = 0; i < 70; i++)
- {
- char templ[] = "conftest.mkstemp/coXXXXXX";
- int (*mkstemp_function) (char *) = mkstemp;
- int fd = mkstemp_function (templ);
- if (fd < 0)
- result |= 1;
- else
- {
- struct stat st;
- if (lseek (fd, large, SEEK_SET) != large)
- result |= 2;
- if (fstat (fd, &st) < 0)
- result |= 4;
- else if (st.st_mode & 0077)
- result |= 8;
- if (close (fd))
- result |= 16;
- }
- }
- return result;
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_run "$LINENO"; then :
- gl_cv_func_working_mkstemp=yes
-else
- gl_cv_func_working_mkstemp=no
-fi
-rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
- conftest.$ac_objext conftest.beam conftest.$ac_ext
-fi
- rm -rf conftest.mkstemp
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gl_cv_func_working_mkstemp" >&5
-$as_echo "$gl_cv_func_working_mkstemp" >&6; }
- case "$gl_cv_func_working_mkstemp" in
- *yes) ;;
- *)
- REPLACE_MKSTEMP=1
- ;;
- esac
- else
- HAVE_MKSTEMP=0
- fi
- if test $HAVE_MKSTEMP = 0 || test $REPLACE_MKSTEMP = 1; then
+ gl_LIBOBJS="$gl_LIBOBJS mkostemp.$ac_objext"
+ fi
- gl_LIBOBJS="$gl_LIBOBJS mkstemp.$ac_objext"
+cat >>confdefs.h <<_ACEOF
+#define GNULIB_MKOSTEMP 1
+_ACEOF
- fi
- GNULIB_MKSTEMP=1
+ GNULIB_MKOSTEMP=1
-$as_echo "#define GNULIB_TEST_MKSTEMP 1" >>confdefs.h
+$as_echo "#define GNULIB_TEST_MKOSTEMP 1" >>confdefs.h
diff --git a/gdb/gnulib/import/Makefile.am b/gdb/gnulib/import/Makefile.am
index d1bee0bc02..608c2c725c 100644
--- a/gdb/gnulib/import/Makefile.am
+++ b/gdb/gnulib/import/Makefile.am
@@ -21,7 +21,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
AUTOMAKE_OPTIONS = 1.9.6 gnits
@@ -1223,14 +1223,14 @@ EXTRA_libgnu_a_SOURCES += mkdir.c
## end gnulib module mkdir
-## begin gnulib module mkstemp
+## begin gnulib module mkostemp
-EXTRA_DIST += mkstemp.c
+EXTRA_DIST += mkostemp.c
-EXTRA_libgnu_a_SOURCES += mkstemp.c
+EXTRA_libgnu_a_SOURCES += mkostemp.c
-## end gnulib module mkstemp
+## end gnulib module mkostemp
## begin gnulib module msvc-inval
diff --git a/gdb/gnulib/import/Makefile.in b/gdb/gnulib/import/Makefile.in
index 7e4d278d91..8b0487ccc7 100644
--- a/gdb/gnulib/import/Makefile.in
+++ b/gdb/gnulib/import/Makefile.in
@@ -35,7 +35,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
@@ -192,7 +192,7 @@ am__aclocal_m4_deps = $(top_srcdir)/import/m4/00gnulib.m4 \
$(top_srcdir)/import/m4/mempcpy.m4 \
$(top_srcdir)/import/m4/memrchr.m4 \
$(top_srcdir)/import/m4/mkdir.m4 \
- $(top_srcdir)/import/m4/mkstemp.m4 \
+ $(top_srcdir)/import/m4/mkostemp.m4 \
$(top_srcdir)/import/m4/mmap-anon.m4 \
$(top_srcdir)/import/m4/mode_t.m4 \
$(top_srcdir)/import/m4/msvc-inval.m4 \
@@ -1515,7 +1515,7 @@ EXTRA_DIST = m4/gnulib-cache.m4 alloca.c alloca.in.h arpa_inet.in.h \
malloca.valgrind math.in.h mbrtowc.c mbsinit.c \
mbsrtowcs-impl.h mbsrtowcs-state.c mbsrtowcs.c memchr.c \
memchr.valgrind memmem.c str-two-way.h mempcpy.c memrchr.c \
- mkdir.c mkstemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
+ mkdir.c mkostemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
msvc-nothrow.h netinet_in.in.h open.c openat.c openat.h \
dirent-private.h opendir.c pathmax.h rawmemchr.c \
rawmemchr.valgrind dirent-private.h readdir.c readlink.c \
@@ -1587,11 +1587,11 @@ EXTRA_libgnu_a_SOURCES = alloca.c openat-proc.c canonicalize-lgpl.c \
gettimeofday.c glob.c inet_ntop.c isnan.c isnand.c isnan.c \
isnanl.c lstat.c malloc.c mbrtowc.c mbsinit.c \
mbsrtowcs-state.c mbsrtowcs.c memchr.c memmem.c mempcpy.c \
- memrchr.c mkdir.c mkstemp.c msvc-inval.c msvc-nothrow.c open.c \
- openat.c opendir.c rawmemchr.c readdir.c readlink.c realloc.c \
- rename.c rewinddir.c rmdir.c secure_getenv.c setenv.c stat.c \
- strchrnul.c strdup.c strerror.c strerror-override.c strstr.c \
- strtok_r.c unsetenv.c
+ memrchr.c mkdir.c mkostemp.c msvc-inval.c msvc-nothrow.c \
+ open.c openat.c opendir.c rawmemchr.c readdir.c readlink.c \
+ realloc.c rename.c rewinddir.c rmdir.c secure_getenv.c \
+ setenv.c stat.c strchrnul.c strdup.c strerror.c \
+ strerror-override.c strstr.c strtok_r.c unsetenv.c
# Use this preprocessor expression to decide whether #include_next works.
# Do not rely on a 'configure'-time test for this, since the expression
@@ -1722,7 +1722,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mempcpy.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memrchr.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdir.Po@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkstemp.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkostemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-inval.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-nothrow.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/open.Po@am__quote@
diff --git a/gdb/gnulib/import/m4/gnulib-cache.m4 b/gdb/gnulib/import/m4/gnulib-cache.m4
index 442aad5563..8cefb67be7 100644
--- a/gdb/gnulib/import/m4/gnulib-cache.m4
+++ b/gdb/gnulib/import/m4/gnulib-cache.m4
@@ -27,7 +27,7 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
# Specification in the form of a few gnulib-tool.m4 macro invocations:
gl_LOCAL_DIR([])
@@ -48,7 +48,7 @@ gl_MODULES([
memchr
memmem
mkdir
- mkstemp
+ mkostemp
pathmax
rawmemchr
readlink
diff --git a/gdb/gnulib/import/m4/gnulib-comp.m4 b/gdb/gnulib/import/m4/gnulib-comp.m4
index 21e4383c34..2c55958eb7 100644
--- a/gdb/gnulib/import/m4/gnulib-comp.m4
+++ b/gdb/gnulib/import/m4/gnulib-comp.m4
@@ -121,7 +121,7 @@ AC_DEFUN([gl_EARLY],
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
- # Code from module mkstemp:
+ # Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
# Code from module multiarch:
@@ -444,12 +444,13 @@ AC_DEFUN([gl_INIT],
if test $REPLACE_MKDIR = 1; then
AC_LIBOBJ([mkdir])
fi
- gl_FUNC_MKSTEMP
- if test $HAVE_MKSTEMP = 0 || test $REPLACE_MKSTEMP = 1; then
- AC_LIBOBJ([mkstemp])
- gl_PREREQ_MKSTEMP
+ gl_FUNC_MKOSTEMP
+ if test $HAVE_MKOSTEMP = 0; then
+ AC_LIBOBJ([mkostemp])
+ gl_PREREQ_MKOSTEMP
fi
- gl_STDLIB_MODULE_INDICATOR([mkstemp])
+ gl_MODULE_INDICATOR([mkostemp])
+ gl_STDLIB_MODULE_INDICATOR([mkostemp])
AC_REQUIRE([gl_MSVC_INVAL])
if test $HAVE_MSVC_INVALID_PARAMETER_HANDLER = 1; then
AC_LIBOBJ([msvc-inval])
@@ -844,7 +845,7 @@ AC_DEFUN([gl_FILE_LIST], [
lib/mempcpy.c
lib/memrchr.c
lib/mkdir.c
- lib/mkstemp.c
+ lib/mkostemp.c
lib/msvc-inval.c
lib/msvc-inval.h
lib/msvc-nothrow.c
@@ -991,7 +992,7 @@ AC_DEFUN([gl_FILE_LIST], [
m4/mempcpy.m4
m4/memrchr.m4
m4/mkdir.m4
- m4/mkstemp.m4
+ m4/mkostemp.m4
m4/mmap-anon.m4
m4/mode_t.m4
m4/msvc-inval.m4
diff --git a/gdb/gnulib/import/m4/mkostemp.m4 b/gdb/gnulib/import/m4/mkostemp.m4
new file mode 100644
index 0000000000..1f44a0390a
--- /dev/null
+++ b/gdb/gnulib/import/m4/mkostemp.m4
@@ -0,0 +1,23 @@
+# mkostemp.m4 serial 2
+dnl Copyright (C) 2009-2016 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_MKOSTEMP],
+[
+ AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+
+ dnl Persuade glibc <stdlib.h> to declare mkostemp().
+ AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
+
+ AC_CHECK_FUNCS_ONCE([mkostemp])
+ if test $ac_cv_func_mkostemp != yes; then
+ HAVE_MKOSTEMP=0
+ fi
+])
+
+# Prerequisites of lib/mkostemp.c.
+AC_DEFUN([gl_PREREQ_MKOSTEMP],
+[
+])
diff --git a/gdb/gnulib/import/m4/mkstemp.m4 b/gdb/gnulib/import/m4/mkstemp.m4
deleted file mode 100644
index 131e4a7b26..0000000000
--- a/gdb/gnulib/import/m4/mkstemp.m4
+++ /dev/null
@@ -1,82 +0,0 @@
-#serial 23
-
-# Copyright (C) 2001, 2003-2007, 2009-2016 Free Software Foundation, Inc.
-# This file is free software; the Free Software Foundation
-# gives unlimited permission to copy and/or distribute it,
-# with or without modifications, as long as this notice is preserved.
-
-# On some hosts (e.g., HP-UX 10.20, SunOS 4.1.4, Solaris 2.5.1), mkstemp has a
-# silly limit that it can create no more than 26 files from a given template.
-# Other systems lack mkstemp altogether.
-# On OSF1/Tru64 V4.0F, the system-provided mkstemp function can create
-# only 32 files per process.
-# On some hosts, mkstemp creates files with mode 0666, which is a security
-# problem and a violation of POSIX 2008.
-# On systems like the above, arrange to use the replacement function.
-AC_DEFUN([gl_FUNC_MKSTEMP],
-[
- AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
-
- AC_CHECK_FUNCS_ONCE([mkstemp])
- if test $ac_cv_func_mkstemp = yes; then
- AC_CACHE_CHECK([for working mkstemp],
- [gl_cv_func_working_mkstemp],
- [
- mkdir conftest.mkstemp
- AC_RUN_IFELSE(
- [AC_LANG_PROGRAM(
- [AC_INCLUDES_DEFAULT],
- [[int result = 0;
- int i;
- off_t large = (off_t) 4294967295u;
- if (large < 0)
- large = 2147483647;
- umask (0);
- for (i = 0; i < 70; i++)
- {
- char templ[] = "conftest.mkstemp/coXXXXXX";
- int (*mkstemp_function) (char *) = mkstemp;
- int fd = mkstemp_function (templ);
- if (fd < 0)
- result |= 1;
- else
- {
- struct stat st;
- if (lseek (fd, large, SEEK_SET) != large)
- result |= 2;
- if (fstat (fd, &st) < 0)
- result |= 4;
- else if (st.st_mode & 0077)
- result |= 8;
- if (close (fd))
- result |= 16;
- }
- }
- return result;]])],
- [gl_cv_func_working_mkstemp=yes],
- [gl_cv_func_working_mkstemp=no],
- [case "$host_os" in
- # Guess yes on glibc systems.
- *-gnu*) gl_cv_func_working_mkstemp="guessing yes" ;;
- # If we don't know, assume the worst.
- *) gl_cv_func_working_mkstemp="guessing no" ;;
- esac
- ])
- rm -rf conftest.mkstemp
- ])
- case "$gl_cv_func_working_mkstemp" in
- *yes) ;;
- *)
- REPLACE_MKSTEMP=1
- ;;
- esac
- else
- HAVE_MKSTEMP=0
- fi
-])
-
-# Prerequisites of lib/mkstemp.c.
-AC_DEFUN([gl_PREREQ_MKSTEMP],
-[
-])
diff --git a/gdb/gnulib/import/mkstemp.c b/gdb/gnulib/import/mkostemp.c
similarity index 79%
rename from gdb/gnulib/import/mkstemp.c
rename to gdb/gnulib/import/mkostemp.c
index 90ed78e49e..31c3e4837f 100644
--- a/gdb/gnulib/import/mkstemp.c
+++ b/gdb/gnulib/import/mkostemp.c
@@ -24,7 +24,7 @@
#if !_LIBC
# include "tempname.h"
# define __gen_tempname gen_tempname
-# ifndef __GT_FILE
+# ifndef __GTFILE
# define __GT_FILE GT_FILE
# endif
#endif
@@ -38,13 +38,9 @@
/* Generate a unique temporary file name from XTEMPLATE.
The last six characters of XTEMPLATE must be "XXXXXX";
they are replaced with a string that makes the file name unique.
- Then open the file and return a fd.
-
- If you are creating temporary files which will later be removed,
- consider using the clean-temp module, which avoids several pitfalls
- of using mkstemp directly. */
+ Then open the file and return a fd. */
int
-mkstemp (char *xtemplate)
+mkostemp (char *xtemplate, int flags)
{
- return __gen_tempname (xtemplate, 0, 0, __GT_FILE);
+ return __gen_tempname (xtemplate, 0, flags, __GT_FILE);
}
diff --git a/gdb/gnulib/import/stdlib.in.h b/gdb/gnulib/import/stdlib.in.h
index db3253bd97..8f803a2ea3 100644
--- a/gdb/gnulib/import/stdlib.in.h
+++ b/gdb/gnulib/import/stdlib.in.h
@@ -87,9 +87,10 @@ struct random_data
# endif
#endif
-#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
+#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_MKOSTEMP@ || @GNULIB_MKOSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
/* On Mac OS X 10.3, only <unistd.h> declares mkstemp. */
/* On Mac OS X 10.5, only <unistd.h> declares mkstemps. */
+/* On Mac OS X 10.13, only <unistd.h> declares mkostemp and mkostemps. */
/* On Cygwin 1.7.1, only <unistd.h> declares getsubopt. */
/* But avoid namespace pollution on glibc systems and native Windows. */
# include <unistd.h>
diff --git a/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch b/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
new file mode 100644
index 0000000000..35f917fac4
--- /dev/null
+++ b/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
@@ -0,0 +1,38 @@
+From 6954995dd32ea98a1973df31f411f3996bb47dfb Mon Sep 17 00:00:00 2001
+From: Tom Tromey <tom@tromey.com>
+Date: Mon, 1 Oct 2018 14:57:45 -0600
+Subject: [PATCH] mkostemp, mkostemps: Fix compilation error in C++ mode on Mac
+ OS X.
+
+Attempting to use the mkostemp module in gdb caused a build failure
+when using the C++ namespace feature, because mkostemp was not
+declared. On OS X, mkostemp is declared in unistd.h, so this patch
+extends the existing special case in stdlib.in.h to cover mkostemp and
+mkostemps.
+
+* lib/stdlib.in.h: Include <unistd.h> for mkostemp and mkostemps
+on OS X.
+---
+ ChangeLog | 6 ++++++
+ lib/stdlib.in.h | 3 ++-
+ 2 files changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/gdb/gnulib/import/stdlib.in.h b/gdb/gnulib/import/stdlib.in.h
+index db3253bd97..8f803a2ea3 100644
+--- a/gdb/gnulib/import/stdlib.in.h
++++ b/gdb/gnulib/import/stdlib.in.h
+@@ -87,9 +87,10 @@ struct random_data
+ # endif
+ #endif
+
+-#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
++#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_MKOSTEMP@ || @GNULIB_MKOSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
+ /* On Mac OS X 10.3, only <unistd.h> declares mkstemp. */
+ /* On Mac OS X 10.5, only <unistd.h> declares mkstemps. */
++/* On Mac OS X 10.13, only <unistd.h> declares mkostemp and mkostemps. */
+ /* On Cygwin 1.7.1, only <unistd.h> declares getsubopt. */
+ /* But avoid namespace pollution on glibc systems and native Windows. */
+ # include <unistd.h>
+--
+2.19.0
+
diff --git a/gdb/gnulib/update-gnulib.sh b/gdb/gnulib/update-gnulib.sh
index 09933ab9fe..5129450577 100755
--- a/gdb/gnulib/update-gnulib.sh
+++ b/gdb/gnulib/update-gnulib.sh
@@ -46,7 +46,7 @@ IMPORTED_GNULIB_MODULES="\
memchr \
memmem \
mkdir \
- mkstemp \
+ mkostemp \
pathmax \
rawmemchr \
readlink \
@@ -169,6 +169,7 @@ apply_patches ()
}
apply_patches "patches/0001-Fix-PR-gdb-23558-Use-system-s-getcwd-when-cross-comp.patch"
+apply_patches "patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch"
# Regenerate all necessary files...
aclocal -Iimport/m4 &&
diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c
index d5c0d30abb..fb6a0d675d 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -19,6 +19,7 @@
#include "defs.h"
+#include "common/filestuff.h"
#include "common/scoped_fd.h"
#include "config.h"
#include "selftest.h"
@@ -31,7 +32,7 @@ static void
test_destroy ()
{
char filename[] = "scoped_fd-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
unlink (filename);
@@ -50,7 +51,7 @@ static void
test_release ()
{
char filename[] = "scoped_fd-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
unlink (filename);
diff --git a/gdb/unittests/scoped_mmap-selftests.c b/gdb/unittests/scoped_mmap-selftests.c
index b181e02f50..75d1aeda8a 100644
--- a/gdb/unittests/scoped_mmap-selftests.c
+++ b/gdb/unittests/scoped_mmap-selftests.c
@@ -19,6 +19,7 @@
#include "defs.h"
+#include "common/filestuff.h"
#include "common/scoped_mmap.h"
#include "config.h"
@@ -88,7 +89,7 @@ static void
test_normal ()
{
char filename[] = "scoped_mmapped_file-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
SELF_CHECK (write (fd, "Hello!", 7) == 7);
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 5/6] Do not reopen temporary files
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
` (2 preceding siblings ...)
2018-10-03 21:02 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
2018-10-03 21:02 ` [PATCH v2 4/6] Use mkostemp, not mkstemp Tom Tromey
2018-10-03 21:02 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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-03 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.
---
gdb/ChangeLog | 8 +++++
gdb/common/scoped_fd.h | 13 +++++++
gdb/dwarf-index-write.c | 56 ++++++++++++++---------------
gdb/unittests/scoped_fd-selftests.c | 17 +++++++++
4 files changed, 65 insertions(+), 29 deletions(-)
diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h
index c2125bd1af..d20e18a2c0 100644
--- a/gdb/common/scoped_fd.h
+++ b/gdb/common/scoped_fd.h
@@ -21,6 +21,7 @@
#define SCOPED_FD_H
#include <unistd.h>
+#include "filestuff.h"
/* A smart-pointer-like class to automatically close a file descriptor. */
@@ -43,6 +44,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 3032798426..6ba1f87623 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<scoped_fd> out_file_fd
- (gdb::in_place, gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY));
- 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. 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"));
- 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,45 +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<scoped_fd> out_file_str_fd
- (gdb::in_place, gdb_mkostemp_cloexec (filename_str_temp.data (),
- O_BINARY));
- if (out_file_str_fd->get () == -1)
+ /* As above, arrange to unlink the file only after the file
+ descriptor has been closed. */
+ gdb::optional<gdb::unlinker> unlink_file_str;
+ 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"));
- 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 fb6a0d675d..6a9c727477 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -65,12 +65,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_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. */
static void
run_tests ()
{
test_destroy ();
test_release ();
+ test_to_file ();
}
} /* namespace scoped_fd */
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
2018-10-03 21:02 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently make_temp_filename is a function local to
write_psymtabs_to_index. This patch moves it to pathstuff.c so that
it can be used from other places in gdb.
gdb/ChangeLog
2018-10-03 Tom Tromey <tom@tromey.com>
* dwarf-index-write.c (write_psymtabs_to_index): Move
make_temp_filename to common/pathstuff.c.
* common/pathstuff.h (make_temp_filename): Declare.
* common/pathstuff.c (make_temp_filename): New function, moved
from dwarf-index-write.c.
---
gdb/ChangeLog | 8 ++++++++
gdb/common/pathstuff.c | 11 +++++++++++
gdb/common/pathstuff.h | 7 +++++++
gdb/dwarf-index-write.c | 11 +----------
4 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 6d8e53f4e1..48ff861eda 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -202,3 +202,14 @@ get_shell ()
return ret;
}
+
+/* See common/pathstuff.h. */
+
+gdb::char_vector
+make_temp_filename (const std::string &f)
+{
+ gdb::char_vector filename_temp (f.length () + 8);
+ strcpy (filename_temp.data (), f.c_str ());
+ strcat (filename_temp.data () + f.size (), "-XXXXXX");
+ return filename_temp;
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 40fbda8d85..14fab04805 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -20,6 +20,8 @@
#ifndef PATHSTUFF_H
#define PATHSTUFF_H
+#include "common/byte-vector.h"
+
/* Path utilities. */
/* Return the real path of FILENAME, expanding all the symbolic links.
@@ -68,4 +70,9 @@ extern std::string get_standard_cache_dir ();
extern const char *get_shell ();
+/* Make a filename suitable to pass to mkstemp based on F (e.g.
+ /tmp/foo -> /tmp/foo-XXXXXX). */
+
+extern gdb::char_vector make_temp_filename (const std::string &f);
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index 252032161f..dc8660e734 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -24,6 +24,7 @@
#include "common/byte-vector.h"
#include "common/filestuff.h"
#include "common/gdb_unlinker.h"
+#include "common/pathstuff.h"
#include "common/scoped_fd.h"
#include "complaints.h"
#include "dwarf-index-common.h"
@@ -1559,16 +1560,6 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
if (stat (objfile_name (objfile), &st) < 0)
perror_with_name (objfile_name (objfile));
- /* Make a filename suitable to pass to mkstemp based on F (e.g.
- /tmp/foo -> /tmp/foo-XXXXXX). */
- auto make_temp_filename = [] (const std::string &f) -> gdb::char_vector
- {
- gdb::char_vector filename_temp (f.length () + 8);
- strcpy (filename_temp.data (), f.c_str ());
- strcat (filename_temp.data () + f.size (), "-XXXXXX");
- return filename_temp;
- };
-
std::string filename (std::string (dir) + SLASH_STRING + basename
+ (index_kind == dw_index_kind::DEBUG_NAMES
? INDEX5_SUFFIX : INDEX4_SUFFIX));
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
2018-10-03 21:02 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
2018-10-03 21:02 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This moves mkdir_recursive from dwarf-index-cache.c to
common/filestuff.c, and also changes it to return a boolean that says
whether or not it worked.
gdb/ChangeLog
2018-10-03 Tom Tromey <tom@tromey.com>
* unittests/mkdir-recursive-selftests.c: New file.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/mkdir-recursive-selftests.c.
* dwarf-index-cache.c (mkdir_recursive): Move to
common/filestuff.c.
(index_cache::store): Check return value of mkdir_recursive.
(create_dir_and_check, test_mkdir_recursive): Move to new file.
(_initialize_index_cache): Don't register test.
* common/filestuff.h (mkdir_recursive): Declare.
* common/filestuff.c (mkdir_recursive): Move from
dwarf-index-cache.c. Return bool.
---
gdb/ChangeLog | 14 +++
gdb/Makefile.in | 1 +
gdb/common/filestuff.c | 45 +++++++++
gdb/common/filestuff.h | 10 ++
gdb/dwarf-index-cache.c | 114 ++--------------------
gdb/unittests/mkdir-recursive-selftests.c | 89 +++++++++++++++++
6 files changed, 165 insertions(+), 108 deletions(-)
create mode 100644 gdb/unittests/mkdir-recursive-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8d780ac758..62d46323e4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -419,6 +419,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/optional-selftests.c \
unittests/parse-connection-spec-selftests.c \
unittests/ptid-selftests.c \
+ unittests/mkdir-recursive-selftests.c \
unittests/rsp-low-selftests.c \
unittests/scoped_fd-selftests.c \
unittests/scoped_mmap-selftests.c \
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index fa10165a7c..763def7643 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -448,3 +448,48 @@ is_regular_file (const char *name, int *errno_ptr)
*errno_ptr = EINVAL;
return false;
}
+
+/* See common/filestuff.h. */
+
+bool
+mkdir_recursive (const char *dir)
+{
+ gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
+ char * const start = holder.get ();
+ char *component_start = start;
+ char *component_end = start;
+
+ while (1)
+ {
+ /* Find the beginning of the next component. */
+ while (*component_start == '/')
+ component_start++;
+
+ /* Are we done? */
+ if (*component_start == '\0')
+ return true;
+
+ /* Find the slash or null-terminator after this component. */
+ component_end = component_start;
+ while (*component_end != '/' && *component_end != '\0')
+ component_end++;
+
+ /* Temporarily replace the slash with a null terminator, so we can create
+ the directory up to this component. */
+ char saved_char = *component_end;
+ *component_end = '\0';
+
+ /* If we get EEXIST and the existing path is a directory, then we're
+ happy. If it exists, but it's a regular file and this is not the last
+ component, we'll fail at the next component. If this is the last
+ component, the caller will fail with ENOTDIR when trying to
+ open/create a file under that path. */
+ if (mkdir (start, 0700) != 0)
+ if (errno != EEXIST)
+ return false;
+
+ /* Restore the overwritten char. */
+ *component_end = saved_char;
+ component_start = component_end;
+ }
+}
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index e9328f5358..ecfc18d600 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -122,4 +122,14 @@ typedef std::unique_ptr<DIR, gdb_dir_deleter> gdb_dir_up;
we're expecting a regular file. */
extern bool is_regular_file (const char *name, int *errno_ptr);
+
+/* A cheap (as in low-quality) recursive mkdir. Try to create all the
+ parents directories up to DIR and DIR itself. Stop if we hit an
+ error along the way. There is no attempt to remove created
+ directories in case of failure.
+
+ Returns false on failure and sets errno. */
+
+extern bool mkdir_recursive (const char *dir);
+
#endif /* FILESTUFF_H */
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 966630b60c..bac98f9db8 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -45,53 +45,6 @@ index_cache global_index_cache;
static cmd_list_element *set_index_cache_prefix_list;
static cmd_list_element *show_index_cache_prefix_list;
-/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
- directories up to DIR and DIR itself. Stop if we hit an error along the way.
- There is no attempt to remove created directories in case of failure. */
-
-static void
-mkdir_recursive (const char *dir)
-{
- gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
- char * const start = holder.get ();
- char *component_start = start;
- char *component_end = start;
-
- while (1)
- {
- /* Find the beginning of the next component. */
- while (*component_start == '/')
- component_start++;
-
- /* Are we done? */
- if (*component_start == '\0')
- return;
-
- /* Find the slash or null-terminator after this component. */
- component_end = component_start;
- while (*component_end != '/' && *component_end != '\0')
- component_end++;
-
- /* Temporarily replace the slash with a null terminator, so we can create
- the directory up to this component. */
- char saved_char = *component_end;
- *component_end = '\0';
-
- /* If we get EEXIST and the existing path is a directory, then we're
- happy. If it exists, but it's a regular file and this is not the last
- component, we'll fail at the next component. If this is the last
- component, the caller will fail with ENOTDIR when trying to
- open/create a file under that path. */
- if (mkdir (start, 0700) != 0)
- if (errno != EEXIST)
- return;
-
- /* Restore the overwritten char. */
- *component_end = saved_char;
- component_start = component_end;
- }
-}
-
/* Default destructor of index_cache_resource. */
index_cache_resource::~index_cache_resource () = default;
@@ -160,7 +113,12 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
TRY
{
/* Try to create the containing directory. */
- mkdir_recursive (m_dir.c_str ());
+ if (!mkdir_recursive (m_dir.c_str ()))
+ {
+ warning (_("index cache: could not make cache directory: %s\n"),
+ safe_strerror (errno));
+ return;
+ }
if (debug_index_cache)
printf_unfiltered ("index cache: writing index cache for objfile %s\n",
@@ -346,62 +304,6 @@ show_index_cache_stats_command (const char *arg, int from_tty)
indent, global_index_cache.n_misses ());
}
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
-namespace selftests
-{
-
-/* Try to create DIR using mkdir_recursive and make sure it exists. */
-
-static bool
-create_dir_and_check (const char *dir)
-{
- mkdir_recursive (dir);
-
- struct stat st;
- if (stat (dir, &st) != 0)
- perror_with_name (("stat"));
-
- return (st.st_mode & S_IFDIR) != 0;
-}
-
-/* Test mkdir_recursive. */
-
-static void
-test_mkdir_recursive ()
-{
- char base[] = "/tmp/gdb-selftests-XXXXXX";
-
- if (mkdtemp (base) == NULL)
- perror_with_name (("mkdtemp"));
-
- /* Try not to leave leftover directories. */
- struct cleanup_dirs {
- cleanup_dirs (const char *base)
- : m_base (base)
- {}
-
- ~cleanup_dirs () {
- rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
- rmdir (string_printf ("%s/a/b", m_base).c_str ());
- rmdir (string_printf ("%s/a", m_base).c_str ());
- rmdir (m_base);
- }
-
- private:
- const char *m_base;
- } cleanup_dirs (base);
-
- std::string dir = string_printf ("%s/a/b", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-
- dir = string_printf ("%s/a/b/c//d/e/", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-}
-}
-#endif /* GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
-
void
_initialize_index_cache ()
{
@@ -456,8 +358,4 @@ _initialize_index_cache ()
When non-zero, debugging output for the index cache is displayed."),
NULL, NULL,
&setdebuglist, &showdebuglist);
-
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
- selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
-#endif
}
diff --git a/gdb/unittests/mkdir-recursive-selftests.c b/gdb/unittests/mkdir-recursive-selftests.c
new file mode 100644
index 0000000000..d501f8e081
--- /dev/null
+++ b/gdb/unittests/mkdir-recursive-selftests.c
@@ -0,0 +1,89 @@
+/* Self tests for scoped_fd for GDB, the GNU debugger.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+
+#include "common/filestuff.h"
+#include "selftest.h"
+
+namespace selftests {
+namespace mkdir_recursive {
+
+/* Try to create DIR using mkdir_recursive and make sure it exists. */
+
+static bool
+create_dir_and_check (const char *dir)
+{
+ ::mkdir_recursive (dir);
+
+ struct stat st;
+ if (stat (dir, &st) != 0)
+ perror_with_name (("stat"));
+
+ return (st.st_mode & S_IFDIR) != 0;
+}
+
+/* Test mkdir_recursive. */
+
+static void
+test ()
+{
+ char base[] = "/tmp/gdb-selftests-XXXXXX";
+
+ if (mkdtemp (base) == NULL)
+ perror_with_name (("mkdtemp"));
+
+ /* Try not to leave leftover directories. */
+ struct cleanup_dirs {
+ cleanup_dirs (const char *base)
+ : m_base (base)
+ {}
+
+ ~cleanup_dirs () {
+ rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b", m_base).c_str ());
+ rmdir (string_printf ("%s/a", m_base).c_str ());
+ rmdir (m_base);
+ }
+
+ private:
+ const char *m_base;
+ } cleanup_dirs (base);
+
+ std::string dir = string_printf ("%s/a/b", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+
+ dir = string_printf ("%s/a/b/c//d/e/", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+}
+
+}
+}
+
+void
+_initialize_mkdir_recursive_selftests ()
+{
+#if defined (HAVE_MKDTEMP)
+ selftests::register_test ("mkdir_recursive",
+ selftests::mkdir_recursive::test);
+#endif
+}
+
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/6] Unify shell-finding logic
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
2018-10-03 21:02 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
2018-10-03 21:02 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
2018-10-03 21:02 ` [PATCH v2 5/6] Do not reopen temporary files Tom Tromey
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed several places in gdb that were using getenv("SHELL") and
then falling back to "/bin/sh" if it returned NULL. This unifies
these into a single function.
gdb/ChangeLog
2018-10-03 Tom Tromey <tom@tromey.com>
* procfs.c (procfs_target::create_inferior): Use get_shell.
* cli/cli-cmds.c (shell_escape): Use get_shell.
* windows-nat.c (windows_nat_target::create_inferior): Use
get_shell.
* common/pathstuff.c (get_shell): New function.
* nat/fork-inferior.c (SHELL_FILE, get_startup_shell): Remove.
(fork_inferior): Use get_shell.
* common/pathstuff.h (get_shell): Declare.
---
gdb/ChangeLog | 11 +++++++++++
gdb/cli/cli-cmds.c | 6 ++----
gdb/common/pathstuff.c | 12 ++++++++++++
gdb/common/pathstuff.h | 4 ++++
gdb/nat/fork-inferior.c | 21 ++-------------------
gdb/procfs.c | 4 ++--
gdb/windows-nat.c | 5 ++---
7 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index c60e5efd0c..37988dd5d7 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -50,6 +50,7 @@
#include "cli/cli-utils.h"
#include "extension.h"
+#include "common/pathstuff.h"
#ifdef TUI
#include "tui/tui.h" /* For tui_active et.al. */
@@ -726,13 +727,10 @@ shell_escape (const char *arg, int from_tty)
if ((pid = vfork ()) == 0)
{
- const char *p, *user_shell;
+ const char *p, *user_shell = get_shell ();
close_most_fds ();
- if ((user_shell = getenv ("SHELL")) == NULL)
- user_shell = "/bin/sh";
-
/* Get the name of the shell for arg0. */
p = lbasename (user_shell);
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 82905c9e68..6d8e53f4e1 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -190,3 +190,15 @@ get_standard_cache_dir ()
return {};
}
+
+/* See common/pathstuff.h. */
+
+const char *
+get_shell ()
+{
+ const char *ret = getenv ("SHELL");
+ if (ret == NULL)
+ ret = "/bin/sh";
+
+ return ret;
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index a43b963651..40fbda8d85 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -64,4 +64,8 @@ extern bool contains_dir_separator (const char *path);
extern std::string get_standard_cache_dir ();
+/* Return the file name of the user's shell. */
+
+extern const char *get_shell ();
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 40cd05a0f8..f1032b43c9 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -24,16 +24,13 @@
#include "target/target.h"
#include "common-inferior.h"
#include "common-gdbthread.h"
+#include "common/pathstuff.h"
#include "signals-state-save-restore.h"
#include "gdb_tilde_expand.h"
#include <vector>
extern char **environ;
-/* Default shell file to be used if 'startup-with-shell' is set but
- $SHELL is not. */
-#define SHELL_FILE "/bin/sh"
-
/* Build the argument vector for execv(3). */
class execv_argv
@@ -265,20 +262,6 @@ execv_argv::init_for_shell (const char *exec_file,
m_argv.push_back (NULL);
}
-/* Return the shell that must be used to startup the inferior. The
- first attempt is the environment variable SHELL; if it is not set,
- then we default to SHELL_FILE. */
-
-static const char *
-get_startup_shell ()
-{
- const char *ret = getenv ("SHELL");
- if (ret == NULL)
- ret = SHELL_FILE;
-
- return ret;
-}
-
/* See nat/fork-inferior.h. */
pid_t
@@ -316,7 +299,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
/* Figure out what shell to start up the user program under. */
if (shell_file == NULL)
- shell_file = get_startup_shell ();
+ shell_file = get_shell ();
gdb_assert (shell_file != NULL);
}
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 6ffe569e69..ca381a71ae 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3035,11 +3035,11 @@ procfs_target::create_inferior (const char *exec_file,
const std::string &allargs,
char **env, int from_tty)
{
- char *shell_file = getenv ("SHELL");
+ const char *shell_file = get_shell ();
char *tryname;
int pid;
- if (shell_file != NULL && strchr (shell_file, '/') == NULL)
+ if (strchr (shell_file, '/') == NULL)
{
/* We will be looking down the PATH to find shell_file. If we
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 0047a26418..8292cf4212 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -68,6 +68,7 @@
#include "complaints.h"
#include "inf-child.h"
#include "gdb_tilde_expand.h"
+#include "common/pathstuff.h"
#define AdjustTokenPrivileges dyn_AdjustTokenPrivileges
#define DebugActiveProcessStop dyn_DebugActiveProcessStop
@@ -2578,9 +2579,7 @@ windows_nat_target::create_inferior (const char *exec_file,
}
else
{
- sh = getenv ("SHELL");
- if (!sh)
- sh = "/bin/sh";
+ sh = get_shell ();
if (cygwin_conv_path (CCP_POSIX_TO_WIN_W, sh, shell, __PMAX) < 0)
error (_("Error starting executable via shell: %d"), errno);
#ifdef __USEWIDE
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 6/6] Cache a copy of the user's shell on macOS
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
` (4 preceding siblings ...)
2018-10-03 21:02 ` [PATCH v2 4/6] Use mkostemp, not mkstemp Tom Tromey
@ 2018-10-03 21:02 ` Tom Tromey
5 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Recent versions of macOS have a feature called System Integrity
Protection. Among other things, This feature prevents ptrace from
tracing certain programs --- for example, the programs in /bin, which
includes typical shells.
This means that startup-with-shell does not work properly. This is PR
cli/23364. Currently there is a workaround in gdb to disable
startup-with-shell when this feature might be in use.
This patch changes gdb to be a bit more precise about when
startup-with-shell will not work, by checking whether the shell
executable is restricted.
If the shell is restricted, then this patch will also cause gdb to
cache a copy of the shell in the gdb cache directory, and then reset
the SHELL environment variable to point to this copy. This lets
startup-with-shell work again.
Tested on High Sierra by trying to start a program using redirection,
and by running startup-with-shell.exp.
gdb/ChangeLog
2018-10-03 Tom Tromey <tom@tromey.com>
PR cli/23364:
* darwin-nat.c (copied_shell): New global.
(may_have_sip): Rename from should_disable_startup_with_shell.
(copy_shell_to_cache, maybe_cache_shell): New functions.
(darwin_nat_target::create_inferior): Update. Use
copied_shell.
---
gdb/ChangeLog | 9 +++
gdb/darwin-nat.c | 151 ++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 153 insertions(+), 7 deletions(-)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index ecc7635d04..1c6f8e061b 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -38,6 +38,7 @@
#include "bfd.h"
#include "bfd/mach-o.h"
+#include <copyfile.h>
#include <sys/ptrace.h>
#include <sys/signal.h>
#include <setjmp.h>
@@ -61,7 +62,11 @@
#include <mach/port.h>
#include "darwin-nat.h"
+#include "filenames.h"
#include "common/filestuff.h"
+#include "common/gdb_unlinker.h"
+#include "common/pathstuff.h"
+#include "common/scoped_fd.h"
#include "nat/fork-inferior.h"
/* Quick overview.
@@ -119,6 +124,10 @@ static int enable_mach_exceptions;
/* Inferior that should report a fake stop event. */
static struct inferior *darwin_inf_fake_stop;
+/* If non-NULL, the shell we actually invoke. See maybe_cache_shell
+ for details. */
+static const char *copied_shell;
+
#define PAGE_TRUNC(x) ((x) & ~(mach_page_size - 1))
#define PAGE_ROUND(x) PAGE_TRUNC((x) + mach_page_size - 1)
@@ -1833,10 +1842,11 @@ darwin_execvp (const char *file, char * const argv[], char * const env[])
posix_spawnp (NULL, argv[0], NULL, &attr, argv, env);
}
-/* Read kernel version, and return TRUE on Sierra or later. */
+/* Read kernel version, and return TRUE if this host may have System
+ Integrity Protection (Sierra or later). */
static bool
-should_disable_startup_with_shell ()
+may_have_sip ()
{
char str[16];
size_t sz = sizeof (str);
@@ -1852,6 +1862,131 @@ should_disable_startup_with_shell ()
return false;
}
+/* A helper for maybe_cache_shell. This copies the shell to the
+ cache. It will throw an exception on any failure. */
+
+static void
+copy_shell_to_cache (const char *shell, const std::string &new_name)
+{
+ scoped_fd from_fd (gdb_open_cloexec (shell, O_RDONLY, 0));
+ if (from_fd.get () < 0)
+ error (_("Could not open shell (%s) for reading: %s"),
+ shell, safe_strerror (errno));
+
+ std::string new_dir = ldirname (new_name.c_str ());
+ if (!mkdir_recursive (new_dir.c_str ()))
+ error (_("Could not make cache directory \"%s\": %s"),
+ new_dir.c_str (), safe_strerror (errno));
+
+ gdb::char_vector temp_name = make_temp_filename (new_name);
+ scoped_fd to_fd (gdb_mkostemp_cloexec (&temp_name[0]));
+ gdb::unlinker unlink_file_on_error (temp_name.data ());
+
+ if (to_fd.get () < 0)
+ error (_("Could not open temporary file \"%s\" for writing: %s"),
+ temp_name.data (), safe_strerror (errno));
+
+ if (fcopyfile (from_fd.get (), to_fd.get (), nullptr,
+ COPYFILE_STAT | COPYFILE_DATA) != 0)
+ error (_("Could not copy shell to cache as \"%s\": %s"),
+ temp_name.data (), safe_strerror (errno));
+
+ /* Be sure that the caching is atomic so that we don't get bad
+ results from multiple copies of gdb running at the same time. */
+ if (rename (temp_name.data (), new_name.c_str ()) != 0)
+ error (_("Could not rename shell cache file to \"%s\": %s"),
+ new_name.c_str (), safe_strerror (errno));
+
+ unlink_file_on_error.keep ();
+}
+
+/* If $SHELL is restricted, try to cache a copy. Starting with El
+ Capitan, macOS introduced System Integrity Protection. Among other
+ things, this prevents certain executables from being ptrace'd. In
+ particular, executables in /bin, like most shells, are affected.
+ To work around this, while preserving command-line glob expansion
+ and redirections, gdb will cache a copy of the shell. Return true
+ if all is well -- either the shell is not subject to SIP or it has
+ been successfully cached. Returns false if something failed. */
+
+static bool
+maybe_cache_shell ()
+{
+ /* SF_RESTRICTED is defined in sys/stat.h and lets us determine if a
+ given file is subject to SIP. */
+#ifdef SF_RESTRICTED
+
+ /* If a check fails we want to revert -- maybe the user deleted the
+ cache while gdb was running, or something like that. */
+ copied_shell = nullptr;
+
+ const char *shell = get_shell ();
+ if (!IS_ABSOLUTE_PATH (shell))
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Normally gdb would try to work around this by caching a copy of your shell,\n\
+but because your shell (%s) is not an absolute path, this is being skipped."),
+ shell);
+ return false;
+ }
+
+ struct stat sb;
+ if (stat (shell, &sb) < 0)
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Normally gdb would try to work around this by caching a copy of your shell,\n\
+but because gdb could not stat your shell (%s), this is being skipped.\n\
+The error was: %s"),
+ shell, safe_strerror (errno));
+ return false;
+ }
+
+ if ((sb.st_flags & SF_RESTRICTED) == 0)
+ return true;
+
+ /* Put the copy somewhere like ~/Library/Caches/gdb/bin/sh. */
+ std::string new_name = get_standard_cache_dir ();
+ if (!IS_DIR_SEPARATOR (new_name.back ()) && !IS_ABSOLUTE_PATH (shell))
+ new_name.push_back ('/');
+ new_name.append (shell);
+
+ /* Maybe it was cached by some earlier gdb. */
+ if (stat (new_name.c_str (), &sb) != 0 || !S_ISREG (sb.st_mode))
+ {
+ TRY
+ {
+ copy_shell_to_cache (shell, new_name);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Because `startup-with-shell' is enabled, gdb tried to work around SIP by\n\
+caching a copy of your shell. However, this failed:\n\
+%s\n\
+If you correct the problem, gdb will automatically try again the next time\n\
+you \"run\". To prevent these attempts, you can use:\n\
+ set startup-with-shell off"),
+ ex.message);
+ return false;
+ }
+ END_CATCH
+
+ printf_filtered (_("Note: this version of macOS has System Integrity Protection.\n\
+Because `startup-with-shell' is enabled, gdb has worked around this by\n\
+caching a copy of your shell. The shell used by \"run\" is now:\n\
+ %s\n"),
+ new_name.c_str ());
+ }
+
+ /* We need to make sure that the new name has the correct lifetime. */
+ static std::string saved_shell = std::move (new_name);
+ copied_shell = saved_shell.c_str ();
+
+#endif /* SF_RESTRICTED */
+
+ return true;
+}
+
void
darwin_nat_target::create_inferior (const char *exec_file,
const std::string &allargs,
@@ -1859,16 +1994,18 @@ darwin_nat_target::create_inferior (const char *exec_file,
{
gdb::optional<scoped_restore_tmpl<int>> restore_startup_with_shell;
- if (startup_with_shell && should_disable_startup_with_shell ())
+ if (startup_with_shell && may_have_sip ())
{
- warning (_("startup-with-shell not supported on this macOS version,"
- " disabling it."));
- restore_startup_with_shell.emplace (&startup_with_shell, 0);
+ if (!maybe_cache_shell ())
+ {
+ warning (_("startup-with-shell is now temporarily disabled"));
+ restore_startup_with_shell.emplace (&startup_with_shell, 0);
+ }
}
/* Do the hard work. */
fork_inferior (exec_file, allargs, env, darwin_ptrace_me,
- darwin_ptrace_him, darwin_pre_ptrace, NULL,
+ darwin_ptrace_him, darwin_pre_ptrace, copied_shell,
darwin_execvp);
}
\f
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2 0/6] A different approach to starutp-with-shell on macOS
@ 2018-10-03 21:02 Tom Tromey
2018-10-03 21:02 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Tom Tromey @ 2018-10-03 21:02 UTC (permalink / raw)
To: gdb-patches
Here's v2 of my series to cache the shell on macOS.
I think this addresses all the comments from the previous patch.
It also incorporates a gnulib bug fix that turned out to be needed in
order to use mkostemp.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-31 15:03 ` Tom Tromey
@ 2018-11-01 19:44 ` Simon Marchi
0 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2018-11-01 19:44 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
On 2018-10-31 11:03, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
>>> It would be normal to look at TMPDIR on unix systems.
>
> Simon> Ah, indeed. How is it with this little fixup on top of the
> patch?
>
> Seems reasonable, thanks!
>
> Simon> + char *tmp = getenv ("TMPDIR");
>
> A tiny nit: you can use const char * here.
>
> Tom
Pushed with that fixed, thanks!
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-30 21:04 ` Simon Marchi
@ 2018-10-31 15:03 ` Tom Tromey
2018-11-01 19:44 ` Simon Marchi
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2018-10-31 15:03 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>> It would be normal to look at TMPDIR on unix systems.
Simon> Ah, indeed. How is it with this little fixup on top of the patch?
Seems reasonable, thanks!
Simon> + char *tmp = getenv ("TMPDIR");
A tiny nit: you can use const char * here.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-30 20:55 ` Tom Tromey
@ 2018-10-30 21:04 ` Simon Marchi
2018-10-31 15:03 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2018-10-30 21:04 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-10-30 4:55 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> What do you think about this?
>
> Thanks for doing this.
>
> Simon> /* See common/pathstuff.h. */
>
> Simon> +std::string
> Simon> +get_standard_temp_dir ()
> Simon> +{
> Simon> +#ifdef WIN32
> Simon> + char *tmp = getenv ("TMP");
> Simon> + if (tmp != nullptr)
> Simon> + return tmp;
> Simon> +
> Simon> + tmp = getenv ("TEMP");
> Simon> + if (tmp != nullptr)
> Simon> + return tmp;
> Simon> +
> Simon> + error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
> Simon> +
> Simon> +#else
> Simon> + return "/tmp";
>
> It would be normal to look at TMPDIR on unix systems.
Ah, indeed. How is it with this little fixup on top of the patch?
From e8b9d0eabfb9bd85afcf42af9bdfb6a5bde66fc2 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 30 Oct 2018 17:01:50 -0400
Subject: [PATCH] fixup
---
gdb/common/pathstuff.c | 4 ++++
gdb/common/pathstuff.h | 5 +++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index c0c575f3fb4..166a21593eb 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -208,6 +208,10 @@ get_standard_temp_dir ()
error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
#else
+ char *tmp = getenv ("TMPDIR");
+ if (tmp != nullptr)
+ return tmp;
+
return "/tmp";
#endif
}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 18af733ab98..f29349e8b28 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -68,8 +68,9 @@ extern std::string get_standard_cache_dir ();
/* Get the usual temporary directory for the current platform.
- On Windows, this is the TMP or TEMP environment variable. On the rest,
- it's /tmp.
+ On Windows, this is the TMP or TEMP environment variable.
+
+ On the rest, this is the TMPDIR environment variable, if defined, else /tmp.
Throw an exception on error. */
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-29 22:16 ` Simon Marchi
@ 2018-10-30 20:55 ` Tom Tromey
2018-10-30 21:04 ` Simon Marchi
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2018-10-30 20:55 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> What do you think about this?
Thanks for doing this.
Simon> /* See common/pathstuff.h. */
Simon> +std::string
Simon> +get_standard_temp_dir ()
Simon> +{
Simon> +#ifdef WIN32
Simon> + char *tmp = getenv ("TMP");
Simon> + if (tmp != nullptr)
Simon> + return tmp;
Simon> +
Simon> + tmp = getenv ("TEMP");
Simon> + if (tmp != nullptr)
Simon> + return tmp;
Simon> +
Simon> + error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
Simon> +
Simon> +#else
Simon> + return "/tmp";
It would be normal to look at TMPDIR on unix systems.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-29 16:01 ` Simon Marchi
@ 2018-10-29 22:16 ` Simon Marchi
2018-10-30 20:55 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2018-10-29 22:16 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-10-29 12:00 p.m., Simon Marchi wrote:
> On 2018-10-18 6:30 p.m., Tom Tromey wrote:
>> This moves mkdir_recursive from dwarf-index-cache.c to
>> common/filestuff.c, and also changes it to return a boolean that says
>> whether or not it worked.
>>
>> gdb/ChangeLog
>> 2018-10-18 Tom Tromey <tom@tromey.com>
>>
>> * unittests/mkdir-recursive-selftests.c: New file.
>> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
>> unittests/mkdir-recursive-selftests.c.
>> * dwarf-index-cache.c (mkdir_recursive): Move to
>> common/filestuff.c.
>> (index_cache::store): Check return value of mkdir_recursive.
>> (create_dir_and_check, test_mkdir_recursive): Move to new file.
>> (_initialize_index_cache): Don't register test.
>> * common/filestuff.h (mkdir_recursive): Declare.
>> * common/filestuff.c (mkdir_recursive): Move from
>> dwarf-index-cache.c. Return bool.
>
> I just stumbled on a build failure caused by this patch, when building for mingw:
>
> CXX unittests/mkdir-recursive-selftests.o
> /home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
> /home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
> if (mkdtemp (base) == NULL)
> ^
>
> The function in the original code was guarded by HAVE_MKDTEMP (the register_test
> call still is). So I guess the function should be put back in an ifdef, or we
> should import the mkdtemp gnulib module (I don't remember why I chose not to do
> that).
>
> Simon
>
What do you think about this?
From 4819c8481fe543df991c940137f80a238592c086 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 29 Oct 2018 16:35:01 -0400
Subject: [PATCH] Import mkdtemp gnulib module, fix mingw build
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Building with mingw currently fails:
CXX unittests/mkdir-recursive-selftests.o
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
if (mkdtemp (base) == NULL)
^
Commit
e418a61a67a ("Move mkdir_recursive to common/filestuff.c")
moved this code, but also removed the HAVE_MKDTEMP guard which prevented
the mkdtemp call to be compiled on mingw.
We can either put back the HAVE_MKDTEMP ifdef, or import the gnulib
mkdtemp module, which provides the function for mingw. Since the
mkdir_recursive is susceptible to be used on mingw at some point, I
think it would be nice to have it tested on mingw, so I did the latter.
Once built, I tested it on Windows (copied the resulting gdb.exe on a
Windows machine, ran it, and ran "maint selftest mkdir_recursive"). It
failed, because the temporary directory is hardcoded to "/tmp/...". I
therefore added and used a new get_standard_temp_dir function, which
returns an appropriate temporary directory for the host platform.
gdb/ChangeLog:
* common/pathstuff.c (get_standard_temp_dir): New.
* common/pathstuff.h (get_standard_temp_dir): New.
* config.in: Re-generate.
* configure: Re-generate.
* configure.ac: Don't check for mkdtemp.
* gnulib/aclocal-m4-deps.mk: Re-generate.
* gnulib/aclocal.m4: Re-generate.
* gnulib/config.in: Re-generate.
* gnulib/configure: Re-generate.
* gnulib/import/Makefile.am: Re-generate.
* gnulib/import/Makefile.in: Re-generate.
* gnulib/import/m4/gnulib-cache.m4: Re-generate.
* gnulib/import/m4/gnulib-comp.m4: Re-generate.
* gnulib/import/m4/mkdtemp.m4: New file.
* gnulib/import/mkdtemp.c: New file.
* gnulib/update-gnulib.sh (IMPORTED_GNULIB_MODULES):
Add mkdtemp module.
* unittests/mkdir-recursive-selftests.c (test): Use
get_standard_temp_dir.
(_initialize_mkdir_recursive_selftests): Remove HAVE_MKDTEMP
ifdef.
* compile/compile.c (get_compile_file_tempdir): Likewise.
---
gdb/common/pathstuff.c | 21 ++++++++++
gdb/common/pathstuff.h | 9 +++++
gdb/compile/compile.c | 4 --
gdb/config.in | 3 --
gdb/configure | 2 +-
gdb/configure.ac | 2 +-
gdb/gnulib/aclocal-m4-deps.mk | 1 +
gdb/gnulib/aclocal.m4 | 1 +
gdb/gnulib/config.in | 6 +++
gdb/gnulib/configure | 47 +++++++++++++++++++++++
gdb/gnulib/import/Makefile.am | 11 +++++-
gdb/gnulib/import/Makefile.in | 20 +++++-----
gdb/gnulib/import/m4/gnulib-cache.m4 | 3 +-
gdb/gnulib/import/m4/gnulib-comp.m4 | 9 +++++
gdb/gnulib/import/m4/mkdtemp.m4 | 20 ++++++++++
gdb/gnulib/import/mkdtemp.c | 39 +++++++++++++++++++
gdb/gnulib/update-gnulib.sh | 1 +
gdb/unittests/mkdir-recursive-selftests.c | 15 ++++----
18 files changed, 187 insertions(+), 27 deletions(-)
create mode 100644 gdb/gnulib/import/m4/mkdtemp.m4
create mode 100644 gdb/gnulib/import/mkdtemp.c
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 48ff861edae..c0c575f3fb4 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -193,6 +193,27 @@ get_standard_cache_dir ()
/* See common/pathstuff.h. */
+std::string
+get_standard_temp_dir ()
+{
+#ifdef WIN32
+ char *tmp = getenv ("TMP");
+ if (tmp != nullptr)
+ return tmp;
+
+ tmp = getenv ("TEMP");
+ if (tmp != nullptr)
+ return tmp;
+
+ error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
+
+#else
+ return "/tmp";
+#endif
+}
+
+/* See common/pathstuff.h. */
+
const char *
get_shell ()
{
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index d57aafff079..18af733ab98 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -66,6 +66,15 @@ extern bool contains_dir_separator (const char *path);
extern std::string get_standard_cache_dir ();
+/* Get the usual temporary directory for the current platform.
+
+ On Windows, this is the TMP or TEMP environment variable. On the rest,
+ it's /tmp.
+
+ Throw an exception on error. */
+
+extern std::string get_standard_temp_dir ();
+
/* Return the file name of the user's shell. Normally this comes from
the SHELL environment variable. */
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index ec644a7b5a8..f6f49e0833f 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -395,11 +395,7 @@ get_compile_file_tempdir (void)
strcpy (tname, TEMPLATE);
#undef TEMPLATE
-#ifdef HAVE_MKDTEMP
tempdir_name = mkdtemp (tname);
-#else
- error (_("Command not supported on this host."));
-#endif
if (tempdir_name == NULL)
perror_with_name (_("Could not make temporary directory"));
diff --git a/gdb/config.in b/gdb/config.in
index f0d14143520..deb9d4a996e 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -288,9 +288,6 @@
/* Define to 1 if you have the <memory.h> header file. */
#undef HAVE_MEMORY_H
-/* Define to 1 if you have the `mkdtemp' function. */
-#undef HAVE_MKDTEMP
-
/* Define to 1 if you have a working `mmap' system call. */
#undef HAVE_MMAP
diff --git a/gdb/configure b/gdb/configure
index 3652455322f..0f3ec4520aa 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13320,7 +13320,7 @@ for ac_func in getauxval getrusage getuid getgid \
sigaction sigprocmask sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack mkdtemp setns
+ ptrace64 sigaltstack setns
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/configure.ac b/gdb/configure.ac
index b2343a90e5f..e1ea60660b9 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1359,7 +1359,7 @@ AC_CHECK_FUNCS([getauxval getrusage getuid getgid \
sigaction sigprocmask sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack mkdtemp setns])
+ ptrace64 sigaltstack setns])
AM_LANGINFO_CODESET
GDB_AC_COMMON
diff --git a/gdb/gnulib/aclocal-m4-deps.mk b/gdb/gnulib/aclocal-m4-deps.mk
index 5b2c6cc5ea3..ffa003d7613 100644
--- a/gdb/gnulib/aclocal-m4-deps.mk
+++ b/gdb/gnulib/aclocal-m4-deps.mk
@@ -80,6 +80,7 @@ aclocal_m4_deps = \
import/m4/mempcpy.m4 \
import/m4/memrchr.m4 \
import/m4/mkdir.m4 \
+ import/m4/mkdtemp.m4 \
import/m4/mkostemp.m4 \
import/m4/mmap-anon.m4 \
import/m4/mode_t.m4 \
diff --git a/gdb/gnulib/aclocal.m4 b/gdb/gnulib/aclocal.m4
index 861caf6692c..e480633394d 100644
--- a/gdb/gnulib/aclocal.m4
+++ b/gdb/gnulib/aclocal.m4
@@ -1353,6 +1353,7 @@ m4_include([import/m4/memmem.m4])
m4_include([import/m4/mempcpy.m4])
m4_include([import/m4/memrchr.m4])
m4_include([import/m4/mkdir.m4])
+m4_include([import/m4/mkdtemp.m4])
m4_include([import/m4/mkostemp.m4])
m4_include([import/m4/mmap-anon.m4])
m4_include([import/m4/mode_t.m4])
diff --git a/gdb/gnulib/config.in b/gdb/gnulib/config.in
index d23d208cb28..2f1a5405506 100644
--- a/gdb/gnulib/config.in
+++ b/gdb/gnulib/config.in
@@ -203,6 +203,9 @@
/* Define to 1 when the gnulib module memrchr should be tested. */
#undef GNULIB_TEST_MEMRCHR
+/* Define to 1 when the gnulib module mkdtemp should be tested. */
+#undef GNULIB_TEST_MKDTEMP
+
/* Define to 1 when the gnulib module mkostemp should be tested. */
#undef GNULIB_TEST_MKOSTEMP
@@ -548,6 +551,9 @@
when it succeeds. */
#undef HAVE_MINIMALLY_WORKING_GETCWD
+/* Define to 1 if you have the `mkdtemp' function. */
+#undef HAVE_MKDTEMP
+
/* Define to 1 if you have the 'mkostemp' function. */
#undef HAVE_MKOSTEMP
diff --git a/gdb/gnulib/configure b/gdb/gnulib/configure
index 5d7f8aa004f..340c622cb39 100644
--- a/gdb/gnulib/configure
+++ b/gdb/gnulib/configure
@@ -5841,6 +5841,7 @@ fi
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
+ # Code from module mkdtemp:
# Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
@@ -21891,6 +21892,52 @@ $as_echo "#define FUNC_MKDIR_DOT_BUG 1" >>confdefs.h
fi
+ for ac_func in mkdtemp
+do :
+ ac_fn_c_check_func "$LINENO" "mkdtemp" "ac_cv_func_mkdtemp"
+if test "x$ac_cv_func_mkdtemp" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_MKDTEMP 1
+_ACEOF
+
+fi
+done
+
+ if test $ac_cv_func_mkdtemp = no; then
+ HAVE_MKDTEMP=0
+ fi
+
+ if test $HAVE_MKDTEMP = 0; then
+
+
+
+
+
+
+
+
+ gl_LIBOBJS="$gl_LIBOBJS mkdtemp.$ac_objext"
+
+ :
+
+ fi
+
+
+
+
+
+ GNULIB_MKDTEMP=1
+
+
+
+
+
+$as_echo "#define GNULIB_TEST_MKDTEMP 1" >>confdefs.h
+
+
+
+
+
diff --git a/gdb/gnulib/import/Makefile.am b/gdb/gnulib/import/Makefile.am
index 608c2c725cf..9dca489ee10 100644
--- a/gdb/gnulib/import/Makefile.am
+++ b/gdb/gnulib/import/Makefile.am
@@ -21,7 +21,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
AUTOMAKE_OPTIONS = 1.9.6 gnits
@@ -1223,6 +1223,15 @@ EXTRA_libgnu_a_SOURCES += mkdir.c
## end gnulib module mkdir
+## begin gnulib module mkdtemp
+
+
+EXTRA_DIST += mkdtemp.c
+
+EXTRA_libgnu_a_SOURCES += mkdtemp.c
+
+## end gnulib module mkdtemp
+
## begin gnulib module mkostemp
diff --git a/gdb/gnulib/import/Makefile.in b/gdb/gnulib/import/Makefile.in
index 8b0487ccc75..f433c363485 100644
--- a/gdb/gnulib/import/Makefile.in
+++ b/gdb/gnulib/import/Makefile.in
@@ -35,7 +35,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
@@ -192,6 +192,7 @@ am__aclocal_m4_deps = $(top_srcdir)/import/m4/00gnulib.m4 \
$(top_srcdir)/import/m4/mempcpy.m4 \
$(top_srcdir)/import/m4/memrchr.m4 \
$(top_srcdir)/import/m4/mkdir.m4 \
+ $(top_srcdir)/import/m4/mkdtemp.m4 \
$(top_srcdir)/import/m4/mkostemp.m4 \
$(top_srcdir)/import/m4/mmap-anon.m4 \
$(top_srcdir)/import/m4/mode_t.m4 \
@@ -1515,9 +1516,9 @@ EXTRA_DIST = m4/gnulib-cache.m4 alloca.c alloca.in.h arpa_inet.in.h \
malloca.valgrind math.in.h mbrtowc.c mbsinit.c \
mbsrtowcs-impl.h mbsrtowcs-state.c mbsrtowcs.c memchr.c \
memchr.valgrind memmem.c str-two-way.h mempcpy.c memrchr.c \
- mkdir.c mkostemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
- msvc-nothrow.h netinet_in.in.h open.c openat.c openat.h \
- dirent-private.h opendir.c pathmax.h rawmemchr.c \
+ mkdir.c mkdtemp.c mkostemp.c msvc-inval.c msvc-inval.h \
+ msvc-nothrow.c msvc-nothrow.h netinet_in.in.h open.c openat.c \
+ openat.h dirent-private.h opendir.c pathmax.h rawmemchr.c \
rawmemchr.valgrind dirent-private.h readdir.c readlink.c \
realloc.c rename.c dirent-private.h rewinddir.c rmdir.c \
same-inode.h save-cwd.h secure_getenv.c setenv.c signal.in.h \
@@ -1587,11 +1588,11 @@ EXTRA_libgnu_a_SOURCES = alloca.c openat-proc.c canonicalize-lgpl.c \
gettimeofday.c glob.c inet_ntop.c isnan.c isnand.c isnan.c \
isnanl.c lstat.c malloc.c mbrtowc.c mbsinit.c \
mbsrtowcs-state.c mbsrtowcs.c memchr.c memmem.c mempcpy.c \
- memrchr.c mkdir.c mkostemp.c msvc-inval.c msvc-nothrow.c \
- open.c openat.c opendir.c rawmemchr.c readdir.c readlink.c \
- realloc.c rename.c rewinddir.c rmdir.c secure_getenv.c \
- setenv.c stat.c strchrnul.c strdup.c strerror.c \
- strerror-override.c strstr.c strtok_r.c unsetenv.c
+ memrchr.c mkdir.c mkdtemp.c mkostemp.c msvc-inval.c \
+ msvc-nothrow.c open.c openat.c opendir.c rawmemchr.c readdir.c \
+ readlink.c realloc.c rename.c rewinddir.c rmdir.c \
+ secure_getenv.c setenv.c stat.c strchrnul.c strdup.c \
+ strerror.c strerror-override.c strstr.c strtok_r.c unsetenv.c
# Use this preprocessor expression to decide whether #include_next works.
# Do not rely on a 'configure'-time test for this, since the expression
@@ -1722,6 +1723,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mempcpy.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memrchr.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdir.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdtemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkostemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-inval.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-nothrow.Po@am__quote@
diff --git a/gdb/gnulib/import/m4/gnulib-cache.m4 b/gdb/gnulib/import/m4/gnulib-cache.m4
index 8cefb67be7c..4cbd1a76fa4 100644
--- a/gdb/gnulib/import/m4/gnulib-cache.m4
+++ b/gdb/gnulib/import/m4/gnulib-cache.m4
@@ -27,7 +27,7 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
# Specification in the form of a few gnulib-tool.m4 macro invocations:
gl_LOCAL_DIR([])
@@ -48,6 +48,7 @@ gl_MODULES([
memchr
memmem
mkdir
+ mkdtemp
mkostemp
pathmax
rawmemchr
diff --git a/gdb/gnulib/import/m4/gnulib-comp.m4 b/gdb/gnulib/import/m4/gnulib-comp.m4
index 2c55958eb7e..1700bdd3cf0 100644
--- a/gdb/gnulib/import/m4/gnulib-comp.m4
+++ b/gdb/gnulib/import/m4/gnulib-comp.m4
@@ -121,6 +121,7 @@ AC_DEFUN([gl_EARLY],
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
+ # Code from module mkdtemp:
# Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
@@ -444,6 +445,12 @@ AC_DEFUN([gl_INIT],
if test $REPLACE_MKDIR = 1; then
AC_LIBOBJ([mkdir])
fi
+ gl_FUNC_MKDTEMP
+ if test $HAVE_MKDTEMP = 0; then
+ AC_LIBOBJ([mkdtemp])
+ gl_PREREQ_MKDTEMP
+ fi
+ gl_STDLIB_MODULE_INDICATOR([mkdtemp])
gl_FUNC_MKOSTEMP
if test $HAVE_MKOSTEMP = 0; then
AC_LIBOBJ([mkostemp])
@@ -845,6 +852,7 @@ AC_DEFUN([gl_FILE_LIST], [
lib/mempcpy.c
lib/memrchr.c
lib/mkdir.c
+ lib/mkdtemp.c
lib/mkostemp.c
lib/msvc-inval.c
lib/msvc-inval.h
@@ -992,6 +1000,7 @@ AC_DEFUN([gl_FILE_LIST], [
m4/mempcpy.m4
m4/memrchr.m4
m4/mkdir.m4
+ m4/mkdtemp.m4
m4/mkostemp.m4
m4/mmap-anon.m4
m4/mode_t.m4
diff --git a/gdb/gnulib/import/m4/mkdtemp.m4 b/gdb/gnulib/import/m4/mkdtemp.m4
new file mode 100644
index 00000000000..a2edd395005
--- /dev/null
+++ b/gdb/gnulib/import/m4/mkdtemp.m4
@@ -0,0 +1,20 @@
+# mkdtemp.m4 serial 8
+dnl Copyright (C) 2001-2003, 2006-2007, 2009-2016 Free Software Foundation,
+dnl Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_MKDTEMP],
+[
+ AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+ AC_CHECK_FUNCS([mkdtemp])
+ if test $ac_cv_func_mkdtemp = no; then
+ HAVE_MKDTEMP=0
+ fi
+])
+
+# Prerequisites of lib/mkdtemp.c
+AC_DEFUN([gl_PREREQ_MKDTEMP],
+[:
+])
diff --git a/gdb/gnulib/import/mkdtemp.c b/gdb/gnulib/import/mkdtemp.c
new file mode 100644
index 00000000000..c1b05fd8ada
--- /dev/null
+++ b/gdb/gnulib/import/mkdtemp.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 1999, 2001-2003, 2006-2007, 2009-2016 Free Software
+ Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Extracted from misc/mkdtemp.c. */
+
+#include <config.h>
+
+/* Specification. */
+#include <stdlib.h>
+
+#include "tempname.h"
+
+/* Generate a unique temporary directory from XTEMPLATE.
+ The last six characters of XTEMPLATE must be "XXXXXX";
+ they are replaced with a string that makes the filename unique.
+ The directory is created, mode 700, and its name is returned.
+ (This function comes from OpenBSD.) */
+char *
+mkdtemp (char *xtemplate)
+{
+ if (gen_tempname (xtemplate, 0, 0, GT_DIR))
+ return NULL;
+ else
+ return xtemplate;
+}
diff --git a/gdb/gnulib/update-gnulib.sh b/gdb/gnulib/update-gnulib.sh
index 5129450577d..e67565a0e72 100755
--- a/gdb/gnulib/update-gnulib.sh
+++ b/gdb/gnulib/update-gnulib.sh
@@ -46,6 +46,7 @@ IMPORTED_GNULIB_MODULES="\
memchr \
memmem \
mkdir \
+ mkdtemp \
mkostemp \
pathmax \
rawmemchr \
diff --git a/gdb/unittests/mkdir-recursive-selftests.c b/gdb/unittests/mkdir-recursive-selftests.c
index d501f8e0817..5a5c453ad6a 100644
--- a/gdb/unittests/mkdir-recursive-selftests.c
+++ b/gdb/unittests/mkdir-recursive-selftests.c
@@ -21,6 +21,8 @@
#include "common/filestuff.h"
#include "selftest.h"
+#include "common/byte-vector.h"
+#include "common/pathstuff.h"
namespace selftests {
namespace mkdir_recursive {
@@ -44,9 +46,10 @@ create_dir_and_check (const char *dir)
static void
test ()
{
- char base[] = "/tmp/gdb-selftests-XXXXXX";
+ std::string tmp = get_standard_temp_dir () + "/gdb-selftests";
+ gdb::char_vector base = make_temp_filename (tmp);
- if (mkdtemp (base) == NULL)
+ if (mkdtemp (base.data ()) == NULL)
perror_with_name (("mkdtemp"));
/* Try not to leave leftover directories. */
@@ -66,12 +69,12 @@ test ()
private:
const char *m_base;
- } cleanup_dirs (base);
+ } cleanup_dirs (base.data ());
- std::string dir = string_printf ("%s/a/b", base);
+ std::string dir = string_printf ("%s/a/b", base.data ());
SELF_CHECK (create_dir_and_check (dir.c_str ()));
- dir = string_printf ("%s/a/b/c//d/e/", base);
+ dir = string_printf ("%s/a/b/c//d/e/", base.data ());
SELF_CHECK (create_dir_and_check (dir.c_str ()));
}
@@ -81,9 +84,7 @@ test ()
void
_initialize_mkdir_recursive_selftests ()
{
-#if defined (HAVE_MKDTEMP)
selftests::register_test ("mkdir_recursive",
selftests::mkdir_recursive::test);
-#endif
}
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-18 22:31 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
@ 2018-10-29 16:01 ` Simon Marchi
2018-10-29 22:16 ` Simon Marchi
0 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2018-10-29 16:01 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-10-18 6:30 p.m., Tom Tromey wrote:
> This moves mkdir_recursive from dwarf-index-cache.c to
> common/filestuff.c, and also changes it to return a boolean that says
> whether or not it worked.
>
> gdb/ChangeLog
> 2018-10-18 Tom Tromey <tom@tromey.com>
>
> * unittests/mkdir-recursive-selftests.c: New file.
> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> unittests/mkdir-recursive-selftests.c.
> * dwarf-index-cache.c (mkdir_recursive): Move to
> common/filestuff.c.
> (index_cache::store): Check return value of mkdir_recursive.
> (create_dir_and_check, test_mkdir_recursive): Move to new file.
> (_initialize_index_cache): Don't register test.
> * common/filestuff.h (mkdir_recursive): Declare.
> * common/filestuff.c (mkdir_recursive): Move from
> dwarf-index-cache.c. Return bool.
I just stumbled on a build failure caused by this patch, when building for mingw:
CXX unittests/mkdir-recursive-selftests.o
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
if (mkdtemp (base) == NULL)
^
The function in the original code was guarded by HAVE_MKDTEMP (the register_test
call still is). So I guess the function should be put back in an ifdef, or we
should import the mkdtemp gnulib module (I don't remember why I chose not to do
that).
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell " Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-29 16:01 ` Simon Marchi
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This moves mkdir_recursive from dwarf-index-cache.c to
common/filestuff.c, and also changes it to return a boolean that says
whether or not it worked.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* unittests/mkdir-recursive-selftests.c: New file.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/mkdir-recursive-selftests.c.
* dwarf-index-cache.c (mkdir_recursive): Move to
common/filestuff.c.
(index_cache::store): Check return value of mkdir_recursive.
(create_dir_and_check, test_mkdir_recursive): Move to new file.
(_initialize_index_cache): Don't register test.
* common/filestuff.h (mkdir_recursive): Declare.
* common/filestuff.c (mkdir_recursive): Move from
dwarf-index-cache.c. Return bool.
---
gdb/ChangeLog | 14 +++
gdb/Makefile.in | 1 +
gdb/common/filestuff.c | 45 +++++++++
gdb/common/filestuff.h | 10 ++
gdb/dwarf-index-cache.c | 114 ++--------------------
gdb/unittests/mkdir-recursive-selftests.c | 89 +++++++++++++++++
6 files changed, 165 insertions(+), 108 deletions(-)
create mode 100644 gdb/unittests/mkdir-recursive-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0bb203f45f..4ab0da5c61 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -419,6 +419,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/optional-selftests.c \
unittests/parse-connection-spec-selftests.c \
unittests/ptid-selftests.c \
+ unittests/mkdir-recursive-selftests.c \
unittests/rsp-low-selftests.c \
unittests/scoped_fd-selftests.c \
unittests/scoped_mmap-selftests.c \
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index dfd86f9fbb..d4bd1a8cb8 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -447,3 +447,48 @@ is_regular_file (const char *name, int *errno_ptr)
*errno_ptr = EINVAL;
return false;
}
+
+/* See common/filestuff.h. */
+
+bool
+mkdir_recursive (const char *dir)
+{
+ gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
+ char * const start = holder.get ();
+ char *component_start = start;
+ char *component_end = start;
+
+ while (1)
+ {
+ /* Find the beginning of the next component. */
+ while (*component_start == '/')
+ component_start++;
+
+ /* Are we done? */
+ if (*component_start == '\0')
+ return true;
+
+ /* Find the slash or null-terminator after this component. */
+ component_end = component_start;
+ while (*component_end != '/' && *component_end != '\0')
+ component_end++;
+
+ /* Temporarily replace the slash with a null terminator, so we can create
+ the directory up to this component. */
+ char saved_char = *component_end;
+ *component_end = '\0';
+
+ /* If we get EEXIST and the existing path is a directory, then we're
+ happy. If it exists, but it's a regular file and this is not the last
+ component, we'll fail at the next component. If this is the last
+ component, the caller will fail with ENOTDIR when trying to
+ open/create a file under that path. */
+ if (mkdir (start, 0700) != 0)
+ if (errno != EEXIST)
+ return false;
+
+ /* Restore the overwritten char. */
+ *component_end = saved_char;
+ component_start = component_end;
+ }
+}
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index e9328f5358..ecfc18d600 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -122,4 +122,14 @@ typedef std::unique_ptr<DIR, gdb_dir_deleter> gdb_dir_up;
we're expecting a regular file. */
extern bool is_regular_file (const char *name, int *errno_ptr);
+
+/* A cheap (as in low-quality) recursive mkdir. Try to create all the
+ parents directories up to DIR and DIR itself. Stop if we hit an
+ error along the way. There is no attempt to remove created
+ directories in case of failure.
+
+ Returns false on failure and sets errno. */
+
+extern bool mkdir_recursive (const char *dir);
+
#endif /* FILESTUFF_H */
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 966630b60c..bac98f9db8 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -45,53 +45,6 @@ index_cache global_index_cache;
static cmd_list_element *set_index_cache_prefix_list;
static cmd_list_element *show_index_cache_prefix_list;
-/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
- directories up to DIR and DIR itself. Stop if we hit an error along the way.
- There is no attempt to remove created directories in case of failure. */
-
-static void
-mkdir_recursive (const char *dir)
-{
- gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
- char * const start = holder.get ();
- char *component_start = start;
- char *component_end = start;
-
- while (1)
- {
- /* Find the beginning of the next component. */
- while (*component_start == '/')
- component_start++;
-
- /* Are we done? */
- if (*component_start == '\0')
- return;
-
- /* Find the slash or null-terminator after this component. */
- component_end = component_start;
- while (*component_end != '/' && *component_end != '\0')
- component_end++;
-
- /* Temporarily replace the slash with a null terminator, so we can create
- the directory up to this component. */
- char saved_char = *component_end;
- *component_end = '\0';
-
- /* If we get EEXIST and the existing path is a directory, then we're
- happy. If it exists, but it's a regular file and this is not the last
- component, we'll fail at the next component. If this is the last
- component, the caller will fail with ENOTDIR when trying to
- open/create a file under that path. */
- if (mkdir (start, 0700) != 0)
- if (errno != EEXIST)
- return;
-
- /* Restore the overwritten char. */
- *component_end = saved_char;
- component_start = component_end;
- }
-}
-
/* Default destructor of index_cache_resource. */
index_cache_resource::~index_cache_resource () = default;
@@ -160,7 +113,12 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
TRY
{
/* Try to create the containing directory. */
- mkdir_recursive (m_dir.c_str ());
+ if (!mkdir_recursive (m_dir.c_str ()))
+ {
+ warning (_("index cache: could not make cache directory: %s\n"),
+ safe_strerror (errno));
+ return;
+ }
if (debug_index_cache)
printf_unfiltered ("index cache: writing index cache for objfile %s\n",
@@ -346,62 +304,6 @@ show_index_cache_stats_command (const char *arg, int from_tty)
indent, global_index_cache.n_misses ());
}
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
-namespace selftests
-{
-
-/* Try to create DIR using mkdir_recursive and make sure it exists. */
-
-static bool
-create_dir_and_check (const char *dir)
-{
- mkdir_recursive (dir);
-
- struct stat st;
- if (stat (dir, &st) != 0)
- perror_with_name (("stat"));
-
- return (st.st_mode & S_IFDIR) != 0;
-}
-
-/* Test mkdir_recursive. */
-
-static void
-test_mkdir_recursive ()
-{
- char base[] = "/tmp/gdb-selftests-XXXXXX";
-
- if (mkdtemp (base) == NULL)
- perror_with_name (("mkdtemp"));
-
- /* Try not to leave leftover directories. */
- struct cleanup_dirs {
- cleanup_dirs (const char *base)
- : m_base (base)
- {}
-
- ~cleanup_dirs () {
- rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
- rmdir (string_printf ("%s/a/b", m_base).c_str ());
- rmdir (string_printf ("%s/a", m_base).c_str ());
- rmdir (m_base);
- }
-
- private:
- const char *m_base;
- } cleanup_dirs (base);
-
- std::string dir = string_printf ("%s/a/b", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-
- dir = string_printf ("%s/a/b/c//d/e/", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-}
-}
-#endif /* GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
-
void
_initialize_index_cache ()
{
@@ -456,8 +358,4 @@ _initialize_index_cache ()
When non-zero, debugging output for the index cache is displayed."),
NULL, NULL,
&setdebuglist, &showdebuglist);
-
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
- selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
-#endif
}
diff --git a/gdb/unittests/mkdir-recursive-selftests.c b/gdb/unittests/mkdir-recursive-selftests.c
new file mode 100644
index 0000000000..d501f8e081
--- /dev/null
+++ b/gdb/unittests/mkdir-recursive-selftests.c
@@ -0,0 +1,89 @@
+/* Self tests for scoped_fd for GDB, the GNU debugger.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+
+#include "common/filestuff.h"
+#include "selftest.h"
+
+namespace selftests {
+namespace mkdir_recursive {
+
+/* Try to create DIR using mkdir_recursive and make sure it exists. */
+
+static bool
+create_dir_and_check (const char *dir)
+{
+ ::mkdir_recursive (dir);
+
+ struct stat st;
+ if (stat (dir, &st) != 0)
+ perror_with_name (("stat"));
+
+ return (st.st_mode & S_IFDIR) != 0;
+}
+
+/* Test mkdir_recursive. */
+
+static void
+test ()
+{
+ char base[] = "/tmp/gdb-selftests-XXXXXX";
+
+ if (mkdtemp (base) == NULL)
+ perror_with_name (("mkdtemp"));
+
+ /* Try not to leave leftover directories. */
+ struct cleanup_dirs {
+ cleanup_dirs (const char *base)
+ : m_base (base)
+ {}
+
+ ~cleanup_dirs () {
+ rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b", m_base).c_str ());
+ rmdir (string_printf ("%s/a", m_base).c_str ());
+ rmdir (m_base);
+ }
+
+ private:
+ const char *m_base;
+ } cleanup_dirs (base);
+
+ std::string dir = string_printf ("%s/a/b", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+
+ dir = string_printf ("%s/a/b/c//d/e/", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+}
+
+}
+}
+
+void
+_initialize_mkdir_recursive_selftests ()
+{
+#if defined (HAVE_MKDTEMP)
+ selftests::register_test ("mkdir_recursive",
+ selftests::mkdir_recursive::test);
+#endif
+}
+
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-11-01 19:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 21:02 [PATCH 2 0/6] A different approach to starutp-with-shell on macOS Tom Tromey
2018-10-03 21:02 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
2018-10-03 21:02 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
2018-10-03 21:02 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
2018-10-03 21:02 ` [PATCH v2 5/6] Do not reopen temporary files Tom Tromey
2018-10-03 21:02 ` [PATCH v2 4/6] Use mkostemp, not mkstemp Tom Tromey
2018-10-03 21:02 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell " Tom Tromey
2018-10-18 22:31 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
2018-10-29 16:01 ` Simon Marchi
2018-10-29 22:16 ` Simon Marchi
2018-10-30 20:55 ` Tom Tromey
2018-10-30 21:04 ` Simon Marchi
2018-10-31 15:03 ` Tom Tromey
2018-11-01 19:44 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).