This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [WIP] Remove nested function in elf/dl-lookup.c? (Nested functions considered harmful?)
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 16 Dec 2013 10:11:29 +0530
- Subject: Re: [WIP] Remove nested function in elf/dl-lookup.c? (Nested functions considered harmful?)
- Authentication-results: sourceware.org; auth=none
- References: <52AC8036 dot 7000300 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