This is the mail archive of the mailing list for the glibc 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] Remove nested function in elf/dl-lookup.c.

On 02/12/2014 04:27 PM, Carlos O'Donell wrote:
> On 02/12/2014 03:50 PM, Joseph S. Myers wrote:
>> This patch loses the call to ELF_MACHINE_SYM_NO_MATCH that was previously 
>> in check_match, and also needs to update the comment on the default 
>> definition of ELF_MACHINE_SYM_NO_MATCH to reflect that check_match is no 
>> longer a nested function.
> I have thrown away the existing refactoring and started again,
> and then compared my new version to the old version to ensure
> that previous testing would still be valid.
> This should catch any changes since the last time I refactored.
> I had included Ondrej's changes but had not noticed your changes.
> Git had given me a merge warning for Ondrej's changes for
> __glibc_*likely, but not yours, perhaps I did something wrong there.
> Either way I've rested the following and it looks good.
> OK?
> v2
> - Fixed ELF_MACHINE_SYM_NO_MATCH comment.

Given the objections to nested functions I've gone ahead and
checked this in after additional regression testing on more
targets including ppc64le.

I checked in version 2 which includes the fixups 

commit 8e25d1e7721d8078d8925e325799740dd53a5801
Author: Carlos O'Donell <>
Date:   Fri Feb 28 18:11:06 2014 -0500

    Promote do_lookup_x:check_match to a full function.
    While it may be argued that nested functions make the resulting
    code easier to read, or worse to read the following two bugs
    make it difficult to debug:
    Bug 8300 - no local symbol information within nested or nesting
    Bug 53927 - wrong value for DW_AT_static_link
    Until these are fixed I've made check_match a full function.
    After they are fixed we can resume arguing about the merits
    of nested functions on readability and maintenance.


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