[v2,1/7] common: add scoped_fd
Commit Message
Add a simple helper to automatically close a file descriptor.
2018-01-26 Markus Metzger <markus.t.metzger@intel.com>
gdb/
* common/scoped_fd.h: New.
* unittests/scoped_fd-selftest.c: New.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/scoped_fd-selftest.c.
---
gdb/Makefile.in | 1 +
gdb/common/scoped_fd.h | 60 +++++++++++++++++++++++++
gdb/unittests/scoped_fd-selftests.c | 90 +++++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+)
create mode 100644 gdb/common/scoped_fd.h
create mode 100644 gdb/unittests/scoped_fd-selftests.c
Comments
On Fri, Jan 26, 2018 at 2:14 PM, Markus Metzger
<markus.t.metzger@intel.com> wrote:
> +
> +/* Test that the file descriptor is closed. */
> +static void
> +test_destroy ()
> +{
> + char filename[] = "scoped_fd-selftest-XXXXXX";
> + int fd = mkstemp (filename);
> + SELF_CHECK (fd >= 0);
Hi Markus,
I failed to build this file with i686-w64-mingw32-g++ 4.8.2, shipped
in Ubuntu 14.04.
gdb/unittests/scoped_fd-selftests.c:37:29: error: ‘mkstemp’ was not
declared in this scope
int fd = mkstemp (filename);
^
gdb/unittests/scoped_fd-selftests.c: In function ‘void
selftests::scoped_fd::test_release()’:
gdb/unittests/scoped_fd-selftests.c:56:29: error: ‘mkstemp’ was not
declared in this scope
int fd = mkstemp (filename);
^
There is no such error with i686-w64-mingw32-g++ 5.3.1. Can we use
"open" instead?
Hello Yao,
> There is no such error with i686-w64-mingw32-g++ 5.3.1. Can we use "open"
> instead?
A fixed filename works most of the time but I'd rather use a temporary file if possible.
> > +/* Test that the file descriptor is closed. */ static void
> > +test_destroy () {
> > + char filename[] = "scoped_fd-selftest-XXXXXX";
> > + int fd = mkstemp (filename);
> > + SELF_CHECK (fd >= 0);
>
> Hi Markus,
> I failed to build this file with i686-w64-mingw32-g++ 4.8.2, shipped in Ubuntu
> 14.04.
>
> gdb/unittests/scoped_fd-selftests.c:37:29: error: ‘mkstemp’ was not declared in
> this scope
> int fd = mkstemp (filename);
> ^
This is supposed to be a glibc function. But it is guarded by some feature macros.
Quote from mkstemp(3): "
mkstemp():
_BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 500 || _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
|| /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200112L
mkostemp(): _GNU_SOURCE
mkstemps(): _BSD_SOURCE || _SVID_SOURCE
mkostemps(): _GNU_SOURCE
"
Maybe the newer compiler is setting some macros automatically that the older compiler doesn't set.
Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp() instead of mkstemp()?
If that doesn't help, I'll try to find a Ubuntu 14.04 to reproduce the issue.
thanks,
Markus.
> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: 13 February 2018 17:48
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: GDB Patches <gdb-patches@sourceware.org>
> Subject: Re: [PATCH v2 1/7] common: add scoped_fd
>
> On Fri, Jan 26, 2018 at 2:14 PM, Markus Metzger <markus.t.metzger@intel.com>
> wrote:
> > +
> > +/* Test that the file descriptor is closed. */ static void
> > +test_destroy () {
> > + char filename[] = "scoped_fd-selftest-XXXXXX";
> > + int fd = mkstemp (filename);
> > + SELF_CHECK (fd >= 0);
>
> Hi Markus,
> I failed to build this file with i686-w64-mingw32-g++ 4.8.2, shipped in Ubuntu
> 14.04.
>
> gdb/unittests/scoped_fd-selftests.c:37:29: error: ‘mkstemp’ was not declared in
> this scope
> int fd = mkstemp (filename);
> ^
> gdb/unittests/scoped_fd-selftests.c: In function ‘void
> selftests::scoped_fd::test_release()’:
> gdb/unittests/scoped_fd-selftests.c:56:29: error: ‘mkstemp’ was not declared in
> this scope
> int fd = mkstemp (filename);
> ^
>
> There is no such error with i686-w64-mingw32-g++ 5.3.1. Can we use "open"
> instead?
>
> --
> Yao (齐尧)
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
On Tue, Feb 13, 2018 at 5:28 PM, Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
>
> This is supposed to be a glibc function. But it is guarded by some feature macros.
> Quote from mkstemp(3): "
>
> mkstemp():
> _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 500 || _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
> || /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200112L
>
> mkostemp(): _GNU_SOURCE
> mkstemps(): _BSD_SOURCE || _SVID_SOURCE
> mkostemps(): _GNU_SOURCE
> "
>
> Maybe the newer compiler is setting some macros automatically that the older compiler doesn't set.
> Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp() instead of mkstemp()?
>
None of them works with my i686-w64-mingw32-g++ 4.8.2. :(
> If that doesn't help, I'll try to find a Ubuntu 14.04 to reproduce the issue.
>
Hello Yao,
> > Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp() instead
> of mkstemp()?
> >
>
> None of them works with my i686-w64-mingw32-g++ 4.8.2. :(
Thanks for trying. I will look into this.
Regards,
Markus.
> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: 14 February 2018 16:23
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: GDB Patches <gdb-patches@sourceware.org>
> Subject: Re: [PATCH v2 1/7] common: add scoped_fd
>
> On Tue, Feb 13, 2018 at 5:28 PM, Metzger, Markus T
> <markus.t.metzger@intel.com> wrote:
> >
> > This is supposed to be a glibc function. But it is guarded by some feature
> macros.
> > Quote from mkstemp(3): "
> >
> > mkstemp():
> > _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 500 ||
> _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
> > || /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200112L
> >
> > mkostemp(): _GNU_SOURCE
> > mkstemps(): _BSD_SOURCE || _SVID_SOURCE
> > mkostemps(): _GNU_SOURCE
> > "
> >
> > Maybe the newer compiler is setting some macros automatically that the older
> compiler doesn't set.
> > Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp() instead
> of mkstemp()?
> >
>
> None of them works with my i686-w64-mingw32-g++ 4.8.2. :(
>
> > If that doesn't help, I'll try to find a Ubuntu 14.04 to reproduce the issue.
> >
>
> --
> Yao (齐尧)
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Hello Yao,
Looks like they added mkstemp() in 2015 after a few complaints:
https://sourceforge.net/u/jeisele123/mingw-w64/ci/f713f639f6f017371c5176f4deab07d1a92473b4/
How about checking for mkstemp and, if not available, falling back to tmpnam.
Something like this:
#ifndef HAVE_MKSTEMP
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
/* Define a racy version of mkstemp based on tmpnam for systems that do not
support mkstemp. */
static int
mkstemp (char *name)
{
if (strlen (name) < L_tmpnam)
return -1;
name = tmpnam (name);
if (name == nullptr)
return -1;
return open (name);
}
#endif /* HAVE_MKSTEMP */
regards,
markus.
> -----Original Message-----
> From: Metzger, Markus T
> Sent: 14 February 2018 18:26
> To: Yao Qi <qiyaoltc@gmail.com>
> Cc: GDB Patches <gdb-patches@sourceware.org>
> Subject: RE: [PATCH v2 1/7] common: add scoped_fd
>
> Hello Yao,
>
> > > Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp()
> > > instead
> > of mkstemp()?
> > >
> >
> > None of them works with my i686-w64-mingw32-g++ 4.8.2. :(
>
> Thanks for trying. I will look into this.
>
> Regards,
> Markus.
>
> > -----Original Message-----
> > From: Yao Qi [mailto:qiyaoltc@gmail.com]
> > Sent: 14 February 2018 16:23
> > To: Metzger, Markus T <markus.t.metzger@intel.com>
> > Cc: GDB Patches <gdb-patches@sourceware.org>
> > Subject: Re: [PATCH v2 1/7] common: add scoped_fd
> >
> > On Tue, Feb 13, 2018 at 5:28 PM, Metzger, Markus T
> > <markus.t.metzger@intel.com> wrote:
> > >
> > > This is supposed to be a glibc function. But it is guarded by some
> > > feature
> > macros.
> > > Quote from mkstemp(3): "
> > >
> > > mkstemp():
> > > _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 500 ||
> > _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
> > > || /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200112L
> > >
> > > mkostemp(): _GNU_SOURCE
> > > mkstemps(): _BSD_SOURCE || _SVID_SOURCE
> > > mkostemps(): _GNU_SOURCE
> > > "
> > >
> > > Maybe the newer compiler is setting some macros automatically that
> > > the older
> > compiler doesn't set.
> > > Could you try setting, say, _POSIX_C_SOURCE or try using mkostemp()
> > > instead
> > of mkstemp()?
> > >
> >
> > None of them works with my i686-w64-mingw32-g++ 4.8.2. :(
> >
> > > If that doesn't help, I'll try to find a Ubuntu 14.04 to reproduce the issue.
> > >
> >
> > --
> > Yao (齐尧)
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
> From: "Metzger, Markus T" <markus.t.metzger@intel.com>
> CC: GDB Patches <gdb-patches@sourceware.org>
> Date: Mon, 19 Feb 2018 15:28:46 +0000
>
> How about checking for mkstemp and, if not available, falling back to tmpnam.
Why not import the corresponding Gnulib module?
On Mon, Feb 19, 2018 at 3:28 PM, Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
> Hello Yao,
>
> Looks like they added mkstemp() in 2015 after a few complaints:
> https://sourceforge.net/u/jeisele123/mingw-w64/ci/f713f639f6f017371c5176f4deab07d1a92473b4/
>
Thanks for looking into this...
> How about checking for mkstemp and, if not available, falling back to tmpnam.
Can we unconditionally use tmpnam+open which are more portable? I know
it is racy, but mkstemp is only used in a test case. It is overkill to me to do
the configure check or import gnulib module.
If we use mkstemp in gdb source (not test case) one day in the future, I prefer
importing gnulib module. What do you think?
> Something like this:
>
> #ifndef HAVE_MKSTEMP
>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdio.h>
>
> /* Define a racy version of mkstemp based on tmpnam for systems that do not
> support mkstemp. */
> static int
> mkstemp (char *name)
> {
> if (strlen (name) < L_tmpnam)
> return -1;
>
> name = tmpnam (name);
> if (name == nullptr)
> return -1;
>
> return open (name);
> }
>
> #endif /* HAVE_MKSTEMP */
>
@@ -424,6 +424,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/optional-selftests.c \
unittests/ptid-selftests.c \
unittests/rsp-low-selftests.c \
+ unittests/scoped_fd-selftests.c \
unittests/scoped_restore-selftests.c \
unittests/xml-utils-selftests.c
new file mode 100644
@@ -0,0 +1,60 @@
+/* scoped_fd, automatically close a file descriptor
+
+ 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/>. */
+
+#ifndef SCOPED_FD_H
+#define SCOPED_FD_H
+
+#include "config.h"
+
+#ifdef HAVE_UNISTD_H
+
+#include <unistd.h>
+
+/* A smart-pointer-like class to automatically close a file descriptor. */
+
+class scoped_fd
+{
+public:
+ explicit scoped_fd (int fd = -1) noexcept : m_fd (fd) {}
+ ~scoped_fd ()
+ {
+ if (m_fd >= 0)
+ close (m_fd);
+ }
+
+ DISABLE_COPY_AND_ASSIGN (scoped_fd);
+
+ int release () noexcept
+ {
+ int fd = m_fd;
+ m_fd = -1;
+ return fd;
+ }
+
+ int get () const noexcept
+ {
+ return m_fd;
+ }
+
+private:
+ int m_fd;
+};
+
+#endif /* HAVE_UNISTD_H */
+#endif /* SCOPED_FD_H */
new file mode 100644
@@ -0,0 +1,90 @@
+/* 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/scoped_fd.h"
+#include "config.h"
+
+#ifdef HAVE_UNISTD_H
+
+#include "selftest.h"
+
+namespace selftests {
+namespace scoped_fd {
+
+/* Test that the file descriptor is closed. */
+static void
+test_destroy ()
+{
+ char filename[] = "scoped_fd-selftest-XXXXXX";
+ int fd = mkstemp (filename);
+ SELF_CHECK (fd >= 0);
+
+ unlink (filename);
+ errno = 0;
+ {
+ ::scoped_fd sfd (fd);
+
+ SELF_CHECK (sfd.get () == fd);
+ }
+
+ SELF_CHECK (close (fd) == -1 && errno == EBADF);
+}
+
+/* Test that the file descriptor can be released. */
+static void
+test_release ()
+{
+ char filename[] = "scoped_fd-selftest-XXXXXX";
+ int fd = mkstemp (filename);
+ SELF_CHECK (fd >= 0);
+
+ unlink (filename);
+ errno = 0;
+ {
+ ::scoped_fd sfd (fd);
+
+ SELF_CHECK (sfd.release () == fd);
+ }
+
+ SELF_CHECK (close (fd) == 0 || errno != EBADF);
+}
+
+/* Run selftests. */
+static void
+run_tests ()
+{
+ test_destroy ();
+ test_release ();
+}
+
+} /* namespace scoped_fd */
+} /* namespace selftests */
+
+#endif /* HAVE_UNISTD_H */
+
+void
+_initialize_scoped_fd_selftests ()
+{
+#ifdef HAVE_UNISTD_H
+ selftests::register_test ("scoped_fd",
+ selftests::scoped_fd::run_tests);
+#endif
+}