This is the mail archive of the binutils@sourceware.org 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 1/5] New target port: Andes 'nds32'. (bfd)


This patch has some GPLv2 notices in it.  Please change them to GPLv3.

Please use the version of the license notice that says "see 
<http://www.gnu.org/licenses/>" rather than giving an FSF postal address.  
(We really ought to convert existing files in binutils....)

As far as I can see, nds32_elfver_strtab could be const (i.e. "static 
const char *const nds32_elfver_strtab[]" - make the array itself const, 
not just the pointer targets).  The sec_name array in 
nds32_elf_final_sda_base looks like it could be static const.  Probably 
other arrays as well should be static const.

Do you really need your target options to go in global variables (with 
names that don't mention the target at all - remember that it's possible 
to build BFD to support multiple targets, so all exported symbols need to 
mention the target name to avoid conflicts)?  There are lots of options in 
e.g. struct elf32_arm_link_hash_table that seem analogous to those you are 
putting in global variables, which suggests you should be able to put 
yours in the corresponding structure.  (armelf.em then calls 
bfd_elf32_arm_set_target_relocs to pass such configuration information to 
BFD.)  I don't know the use of align_addend, sdata_range, 
sdata_init_range, but certainly they can't be exported with those names 
("nm" on elf32-nds32.o should not reveal any exported symbols whose names 
don't mention nds32), and in general it's better for modifiable data to be 
associated in some way with a BFD instance rather than static if possible.

nds32_insertion_sort uses alloca, but you could have too many relocations 
to fit on the stack - whenever using alloca you need to fallback to 
xmalloc if the size is too big (or simply use xmalloc unconditionally), to 
avoid stack overflow.

You have a "FIXME: This should not be a static variable.".  So fix it.  
(Review all such comments in general, at least to consider whether they do 
still reflect a desired change and whether that change would be easy to 
make now.)

-- 
Joseph S. Myers
joseph@codesourcery.com


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