This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, AArch64, ILP32] 5/5 Improve the debugging experience with the generated file

Hi David,

Thanks for the review.

On 06/21/13 19:52, David Daney wrote:
On 06/21/2013 11:08 AM, Yufeng Zhang wrote:
diff --git a/bfd/ b/bfd/
index 19b3710..0e69d12 100644
--- a/bfd/
+++ b/bfd/
@@ -917,12 +917,14 @@ elf64-target.h : elfxx-target.h

   elf32-aarch64.c : elfnn-aarch64.c
   	rm -f elf32-aarch64.c
-	sed -e s/NN/32/g<  $(srcdir)/elfnn-aarch64.c>
+	echo "#line 1 \"$(srcdir)/elfnn-aarch64.c\"">
+	sed -e s/NN/32/g<  $(srcdir)/elfnn-aarch64.c>>
   	mv -f elf32-aarch64.c

   elf64-aarch64.c : elfnn-aarch64.c
   	rm -f elf64-aarch64.c
-	sed -e s/NN/64/g<  $(srcdir)/elfnn-aarch64.c>
+	echo "#line 1 \"$(srcdir)/elfnn-aarch64.c\"">
+	sed -e s/NN/64/g<  $(srcdir)/elfnn-aarch64.c>>
   	mv -f elf64-aarch64.c

   elf32-ia64.c : elfnn-ia64.c

I really hate this pattern of running the file through sed and
synthesizing the #line.

I know this was built up out of several of the other patches, but I am
commenting on it here.

I understand that you dislike the way of generating c files through 'sed'; there were similar comments received during the internal patch review. I had considered the cpp macro based approach but didn't find it particular helpful in parameterizing the large number of different structure names and functions names that contain elfNN or ElfNN (where NN is either 64 or 32); it will make the source code less readable by defining more macros.

Another argument is that the use of 'sed' is an existing technique in bfd. elf32-target.h and elf64-target.h are generated from elfxx-target.h via 'sed', so is the backend file generation in the IA64 port.

Can you instead move the duplicated code to a file to be included
twice, parameterizing it with one or more preprocessor macros?


#define AARCH64_ABI 32
#include "elfnn-arch64.c"
#undef AARCH64_ABI
#define AARCH64_ABI 64
#include "elfnn-arch64.c"

It looks to me that due to the way bfd is structured it is not feasible to have the ELF32 and ELF64 support defined in the same file. If we really want to go after the cpp macro based approach, we can have two different files include elfnn-aarch64.c with different macro definitions.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]