diff mbox

Don't check HAVE_UNISTD_H

Message ID 20180926113141.21219-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 26, 2018, 11:31 a.m. UTC 
  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(-)

Comments

Simon Marchi Sept. 28, 2018, 9:03 p.m. UTC | #1 
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
Tom Tromey Oct. 1, 2018, 3:33 p.m. UTC | #2 
>>>>> "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
diff mbox

Patch

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",