[WIP] Remove nested function in elf/dl-lookup.c? (Nested functions considered harmful?)

[Carlos O'Donell]

On 12/16/2013 03:56 PM, Roland McGrath wrote:
>> Is it me or does reading do_lookup_x get very very complicated
>> with the nested function check_match which has access to all
>> sorts of function local variables?
>>
>> I'm always suspicious of nested functions like this as bad
>> programming form since it complicates reading the function.
> 
> No, using a nested function like this vastly improves readability.
> Having a function with arguments that are just copies of its callers
> arguments is error-prone and tedious to read through.

I think you're teetering in hyperbole here. Does it really vastly
improve readability? The removal of the nested function creates
a clean interface between the caller and the callee, declares
const-ness, gets it out of the way of the reading of the original
function without having to jump around the nested function.

The only down side appears to be that you have to maintain the
argument list, which is true of any function really, and that's
a cost I would pay to get it out of the way of the function proper.
 
>> Is anyone opposed to rewriting this with a non-nested function
>> that clearly shows which arguments are constant, and which are
>> modified?
> 
> A good way to make that clear is to put the definition of the nested
> function earlier in the scope, where only the things it uses have been
> declared at the definition site.

I still find this difficult to read. Just for my own sake can you
please confirm that you find it easier to read do_lookup_x with
that nested function in place? This is a specific example, we are
not discussing nested functions in general. Please have a look at
the function in question and confirm.

Cheers,
Carlos.