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

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Sat, Dec 14, 2013 at 10:58:46AM -0500, Carlos O'Donell 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.
> 
> Is anyone opposed to rewriting this with a non-nested function
> that clearly shows which arguments are constant, and which are
> modified?

i'd be in favour of this except...

> +/* Utility function for do_lookup_x. The caller is called with undef_name,
> +   ref, version, flags and type_class, and those are passed as the first
> +   five arguments. The caller then computes sym, symidx, strtab, and map
> +   and passes them as the next four arguments. Lastly the caller passes in
> +   versioned_sym and num_versions which are modified by check_match during
> +   the checking process.  */
> +static const ElfW(Sym) *
> +check_match (const char *const undef_name,
> +	     const ElfW(Sym) *const ref,
> +	     const struct r_found_version *const version,
> +	     const int flags,
> +	     const int type_class,
> +	     const ElfW(Sym) *const sym,
> +	     const Elf_Symndx symidx,
> +	     const char *const strtab,
> +	     const struct link_map *const map,
> +	     const ElfW(Sym) **const versioned_sym,
> +	     int *const num_versions)
> +{

... that I find a function with 11 arguments a bit ugly.  Could this
be re-factored somehow to make check_match not depend on so many
parameters?

Siddhesh