[PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
Dave Korn
dave.korn.cygwin@gmail.com
Thu Oct 7 21:42:00 GMT 2010
On 06/10/2010 23:14, Richard Henderson wrote:
> On 09/22/2010 10:31 PM, Dave Korn wrote:
>> - I'd certainly appreciate a second pair of eyes looking at the logic in
>> asymbol_from_plugin_symbol by which I derive the fags etc. for a bfd asymbol
>> from the ld plugin symbol struct.
>
> It does seem to be broken. I see no way you're going to
> be able to generate cigarettes from that code.
LOL, only thing broken there was my brain... I meant flags of course!
>> - return TRUE;
>> + checked_ok = TRUE;
>
> I wonder if a "goto success" wouldn't be cleaner.
Will do.
>> + int fildes = bfd_check_format (entry->the_bfd, bfd_object)
>> + ? open (attempt, O_RDONLY
>> +# ifdef O_BINARY
>> + | O_BINARY
>> +# endif /* O_BINARY */
>> + )
>> + : -1;
>
> You already caught the c99 issue yourself. As for O_BINARY,
> better to put that in sysdeps.h with the rest of the O_FLAGS.
Will do.
>
>> +/* Don't include std headers until after config.h, sysdeps.h etc.,
>> + or you may end up with the wrong bitsize of off_t. */
>> +#include <dlfcn.h>
>
> Better to do this in sysdeps.h, with ifdef protection. I like
> having everything system related done in the one place.
OK. Leaving it here worked out when --enable-plugins was not the default
and there wasn't autoconfigury to detect the dl* functions/headers, but since
I'm adding that (see previous reply) I can easily move this now that there'll
be a HAVE_DLFCN_H for it.
>
>> + asym->section = ldsym->def == LDPK_COMMON
>> + ? bfd_com_section_ptr
>> + : (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
>> + ? bfd_und_section_ptr
>> + : bfd_get_section_by_name (abfd, ".text"));
>
> This is pretty hard to read as-is. Better as
Thanks, will refactor it into a switch as you suggest.
>> + rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
>
> Better I think just as syms + n. I prefer not to increment
> induction variables at non-obvious spots inside the loop.
> In this case we can rely on the compiler producing the ++
> if it really turns out to be useful.
OK, will do. Thanks for reviewing!
cheers,
DaveK
More information about the Binutils
mailing list