Re: [PATCH] Include string.h in elf-bfd.h

[Saagar Jha <saagar@saagarjha.com>]

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

Hi Nick,

Sorry for the delay, I forgot to send the patch to the list ;) Here is an updated one that is largely based on your suggestions. Please let me know if it needs anything else.

Thanks,
Saagar


[-- Attachment #2: Include-string-headers-in-a-configure-test-for-GDB.patch --]
[-- Type: application/octet-stream, Size: 1905 bytes --]

From 0b6c348aff36defee70306eee64b52c9cba1138f Mon Sep 17 00:00:00 2001
From: Saagar Jha <saagar@saagarjha.com>
Date: Tue, 22 Sep 2020 23:52:18 -0700
Subject: [PATCH] Include string headers in a configure test for GDB

Some configure tests include elf-bfd.h by itself and fail because they
can't find prototypes for the string functions in it (which makes GDB
think BFD has no support for ELF files). Normally we'd include sysdep.h,
but for the case of this configure test it's easier to copy the relevant
parts out rather than deal with the redefinitions that would arise were
we to include it directly.

gdb/Changelog:

	* configure: Include string headers in a test that needed it.
---
 gdb/ChangeLog |  4 ++++
 gdb/configure | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ce9cef5f516..23a4e13b38f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-22  Saagar Jha  <saagar@saagarjha.com>
+
+	* configure: Include string headers in a test that needed it.
+
 2020-09-21  Tom Tromey  <tromey@adacore.com>
 
 	* sparc-tdep.c (sparc32_skip_prologue): Use
diff --git a/gdb/configure b/gdb/configure
index a8942ecbd5d..3345320a0fe 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -16771,6 +16771,24 @@ else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdlib.h>
+// This configure test needs string functions, but bfd/sysdep.h isn't included
+// here so we need to copy/paste the relevant part into this test.
+#ifdef STRING_WITH_STRINGS
+#include <string.h>
+#include <strings.h>
+#else
+#ifdef HAVE_STRING_H
+#include <string.h>
+#else
+#ifdef HAVE_STRINGS_H
+#include <strings.h>
+#else
+extern char *strchr ();
+extern char *strrchr ();
+#endif
+#endif
+#endif
+
   #include "bfd.h"
   #include "elf-bfd.h"
 
-- 
2.28.0


[-- Attachment #3: Type: text/plain, Size: 1882 bytes --]



> On Sep 14, 2020, at 02:55, Nick Clifton <nickc@redhat.com> wrote:
> 
> Hi Saagar,
> 
>> Perhaps I’m doing it wrong, but I stuck #include “sysdep.h" at the top of that configure test and it didn’t seem to like it:
> 
>> In file included from conftest.c:171:
>> ../bfd/sysdep.h:26:2: error: sysdep.h must be included in lieu of config.h
>> #error sysdep.h must be included in lieu of config.h
> 
> Ah yes.  For normal code "sysdep,h" is included and this will automatically
> include "config.h" for you.  But with configure tests a lot of the defines
> that are provided by config.h are already set up.  Like this:
> 
>> ../bfd/config.h:310:9: warning: 'PACKAGE' macro redefined [-Wmacro-redefined]
>> conftest.c:26:9: note: previous definition is here
>> #define PACKAGE "gdb"
> 
> So in order to include sysdep.h you must either stop configure from providing
> all of these defines (probably not a good idea) or else undefine them before
> including sysdep.h (also a bad idea) or...
> 
> 
>> configure:16833: $? = 1
>> configure: failed program was:
> [...]
>> | #define HAVE_STRING_H 1
>> | #define HAVE_STRINGS_H 1
> 
> Give up on including sysdep.h, and instead copy the logic from that file that decides
> which string header to include.  IE:
> 
>  #ifdef STRING_WITH_STRINGS
>  #include <string.h>
>  #include <strings.h>
>  #else
>  #ifdef HAVE_STRING_H
>  #include <string.h>
>  #else
>  #ifdef HAVE_STRINGS_H
>  #include <strings.h>
>  #else
>  extern char *strchr ();
>  extern char *strrchr ();
>  #endif
>  #endif
>  #endif
> 
> If you do this I would strongly urge you to add a comment as well, explaining that
> the code has been copied from bfd/sysdep.h and that that header is not included
> directly because it conflicts with the way that configure sets up its defines.
> 
> Cheers
>  Nick
> 
>