public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Don't check HAVE_UNISTD_H
@ 2018-09-26 11:32 Tom Tromey
  2018-09-28 21:03 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2018-09-26 11:32 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed some spots that were checking HAVE_UNISTD_H.  There is no
need to do this, as <unistd.h> is unconditionally included in many
places in gdb.  This sort of cleanup was done once before, in 2013:

    2013-07-01  Pedro Alves  <palves@redhat.com>

	    * defs.h: Don't check HAVE_UNISTD_H before including <unistd.h>.
	    (STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO): Delete.
	    * tracepoint.c: Don't check HAVE_UNISTD_H before including
	    <unistd.h>.

HAVE_UNISTD_H seems to come from gnulib, so there are still mentions
of it in the source.

gdb/ChangeLog
2018-09-26  Tom Tromey  <tom@tromey.com>

	* unittests/scoped_mmap-selftests.c: Don't check HAVE_UNISTD_H.
	* unittests/scoped_fd-selftests.c: Don't check HAVE_UNISTD_H.
	* common/scoped_fd.h: Don't check HAVE_UNISTD_H.
---
 gdb/ChangeLog                         | 6 ++++++
 gdb/common/scoped_fd.h                | 5 -----
 gdb/unittests/scoped_fd-selftests.c   | 7 -------
 gdb/unittests/scoped_mmap-selftests.c | 6 +++---
 4 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h
index a6a8ab9a38..c2125bd1af 100644
--- a/gdb/common/scoped_fd.h
+++ b/gdb/common/scoped_fd.h
@@ -20,10 +20,6 @@
 #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.  */
@@ -56,5 +52,4 @@ private:
   int m_fd;
 };
 
-#endif /* HAVE_UNISTD_H */
 #endif /* SCOPED_FD_H */
diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c
index 4d7454134a..d5c0d30abb 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -21,9 +21,6 @@
 
 #include "common/scoped_fd.h"
 #include "config.h"
-
-#ifdef HAVE_UNISTD_H
-
 #include "selftest.h"
 
 namespace selftests {
@@ -78,13 +75,9 @@ run_tests ()
 } /* 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
 }
diff --git a/gdb/unittests/scoped_mmap-selftests.c b/gdb/unittests/scoped_mmap-selftests.c
index e9d4afdffc..b181e02f50 100644
--- a/gdb/unittests/scoped_mmap-selftests.c
+++ b/gdb/unittests/scoped_mmap-selftests.c
@@ -22,7 +22,7 @@
 #include "common/scoped_mmap.h"
 #include "config.h"
 
-#if defined(HAVE_SYS_MMAN_H) && defined(HAVE_UNISTD_H)
+#if defined(HAVE_SYS_MMAN_H)
 
 #include "selftest.h"
 #include "common/gdb_unlinker.h"
@@ -132,12 +132,12 @@ run_tests ()
 } /* namespace mmap_file */
 } /* namespace selftests */
 
-#endif /* !defined(HAVE_SYS_MMAN_H) || !defined(HAVE_UNISTD_H) */
+#endif /* !defined(HAVE_SYS_MMAN_H) */
 
 void
 _initialize_scoped_mmap_selftests ()
 {
-#if defined(HAVE_SYS_MMAN_H) && defined(HAVE_UNISTD_H)
+#if defined(HAVE_SYS_MMAN_H)
   selftests::register_test ("scoped_mmap",
 			    selftests::scoped_mmap::run_tests);
   selftests::register_test ("mmap_file",
-- 
2.17.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Don't check HAVE_UNISTD_H
  2018-09-26 11:32 [PATCH] Don't check HAVE_UNISTD_H Tom Tromey
@ 2018-09-28 21:03 ` Simon Marchi
  2018-10-01 15:33   ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2018-09-28 21:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2018-09-26 07:31, Tom Tromey wrote:
> I noticed some spots that were checking HAVE_UNISTD_H.  There is no
> need to do this, as <unistd.h> is unconditionally included in many
> places in gdb.  This sort of cleanup was done once before, in 2013:
> 
>     2013-07-01  Pedro Alves  <palves@redhat.com>
> 
> 	    * defs.h: Don't check HAVE_UNISTD_H before including <unistd.h>.
> 	    (STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO): Delete.
> 	    * tracepoint.c: Don't check HAVE_UNISTD_H before including
> 	    <unistd.h>.
> 
> HAVE_UNISTD_H seems to come from gnulib, so there are still mentions
> of it in the source.

LGTM.  Maybe we could even remove all includes of unistd.h except the 
one in defs.h, since defs.h is known to be included first in every 
source file.  But they are harmless so it's probably not worth the 
effort.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Don't check HAVE_UNISTD_H
  2018-09-28 21:03 ` Simon Marchi
@ 2018-10-01 15:33   ` Tom Tromey
  0 siblings, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2018-10-01 15:33 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> LGTM.  Maybe we could even remove all includes of unistd.h except the
Simon> one in defs.h, since defs.h is known to be included first in every
Simon> source file.  But they are harmless so it's probably not worth the
Simon> effort.

Alternatively we could get rid of the one in defs.h and try to have
headers include what they use.  I'm not sure which is better really.
Anyway, I agree that it all seems relatively harmless, so I didn't make
any changes here.

Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-10-01 15:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 11:32 [PATCH] Don't check HAVE_UNISTD_H Tom Tromey
2018-09-28 21:03 ` Simon Marchi
2018-10-01 15:33   ` Tom Tromey

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).