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: ld/2338: objdump -d -l doesn't work correctly


On 15 February 2006 12:30, Dave Korn wrote:

> On 14 February 2006 23:24, H. J. Lu wrote:
> 
>> On Tue, Feb 14, 2006 at 11:54:05PM +0100, Andreas Schwab wrote:
>>> "H. J. Lu" <hjl@lucon.org> writes:
>>> 
>>>> +/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
>>>> +   SECTION.  */
>>> 
>>> Double negation is not easy to not misunderstand.
>>> 
>> 
>> I am trying to say that only FALSE return is reliable since TRUE can
>> return under other conditions. Maybe I should change the function
>> name to function_name_section_mismatch.
> 
>   Or how about wording it the other way round?

  Or how about _I_ pay more attention to the explanation you already gave?
:-O  This is a strange kind-of-inverted predicate function, it's a bit odd; I
would suggest considering the possibility that making it conceptually clearer
may be worth the cost of logical-negating the return value from strcmp (and
changing the default TRUE to a FALSE at the end, of course).  Here's a more
accurate description than my last attempt, anyway:


/*  Return FALSE if FUNC matches a function symbol from section SECTION
  and is found in symbol table SYMBOL.  */


... perhaps you should move the abfd->flags test for a relocatable file
outside the function, because it's a separate test really, and it's the only
reason for passing in the abfd parameter so you could otherwise omit it?  It
seems strange to see a function called "check_function_name" and the first
thing it's doing is talking about whether two symbols can have the same
address and how it relates to whether the object is relocatable or not; I'd
put the comment and the test as part of the if-conditional at the call sites.




    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....


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