remove nested functions from elf/dl-lookup.c
Commit Message
Hi,
Please review the patch that removes a nested function from elf/dl-lookup.c.
This just moves a nested function outside -- no change in parameters.
No regressions in 'make check' on x86_64-linux-gnu (Ubuntu 14.04)
2014-10-06 Kostya Serebryany <konstantin.s.serebryany@gmail.com>
* elf/dl-lookup.c
(enter): New function broken out do_lookup_unique.
(do_lookup_unique): Remove that nested function.
Comments
Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes:
> * elf/dl-lookup.c
> (enter): New function broken out do_lookup_unique.
> (do_lookup_unique): Remove that nested function.
Ok. That can be written like this:
* elf/dl-lookup.c (enter): Move to file scope.
Andreas.
> +/* We have to determine whether we already found a
> + symbol with this name before. If not then we have to
> + add it to the search table. If we already found a
> + definition we have to use it. */
[...]
> - /* We have to determine whether we already found a
> - symbol with this name before. If not then we have to
> - add it to the search table. If we already found a
> - definition we have to use it. */
When you move a comment to a different indentation level, it's worth
hitting M-q just to see if it comes out prettier in fewer lines.
However, in this case the comment should not have been moved.
It's describing the logic of do_lookup_unique, not of enter.
The name "enter" is a bit too generic for a file-scope function.
I fixed these things up and committed for you.
Thanks,
Roland
Thanks!
On Wed, Oct 8, 2014 at 3:18 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> +/* We have to determine whether we already found a
>> + symbol with this name before. If not then we have to
>> + add it to the search table. If we already found a
>> + definition we have to use it. */
> [...]
>> - /* We have to determine whether we already found a
>> - symbol with this name before. If not then we have to
>> - add it to the search table. If we already found a
>> - definition we have to use it. */
>
> When you move a comment to a different indentation level, it's worth
> hitting M-q just to see if it comes out prettier in fewer lines.
>
> However, in this case the comment should not have been moved.
> It's describing the logic of do_lookup_unique, not of enter.
> The name "enter" is a bit too generic for a file-scope function.
>
> I fixed these things up and committed for you.
>
>
> Thanks,
> Roland
@@ -182,6 +182,30 @@ check_match (const char *const undef_name,
return sym;
}
+/* We have to determine whether we already found a
+ symbol with this name before. If not then we have to
+ add it to the search table. If we already found a
+ definition we have to use it. */
+static void
+enter (struct unique_sym *table, size_t size,
+ unsigned int hash, const char *name,
+ const ElfW(Sym) *sym, const struct link_map *map)
+{
+ size_t idx = hash % size;
+ size_t hash2 = 1 + hash % (size - 2);
+ while (table[idx].name != NULL)
+ {
+ idx += hash2;
+ if (idx >= size)
+ idx -= size;
+ }
+
+ table[idx].hashval = hash;
+ table[idx].name = name;
+ table[idx].sym = sym;
+ table[idx].map = map;
+}
+
/* Utility function for do_lookup_x. Lookup an STB_GNU_UNIQUE symbol
in the unique symbol table, creating a new entry if necessary.
Return the matching symbol in RESULT. */
@@ -191,29 +215,6 @@ do_lookup_unique (const char *undef_name, uint_fast32_t new_hash,
int type_class, const ElfW(Sym) *sym, const char *strtab,
const ElfW(Sym) *ref, const struct link_map *undef_map)
{
- /* We have to determine whether we already found a
- symbol with this name before. If not then we have to
- add it to the search table. If we already found a
- definition we have to use it. */
- void enter (struct unique_sym *table, size_t size,
- unsigned int hash, const char *name,
- const ElfW(Sym) *sym, const struct link_map *map)
- {
- size_t idx = hash % size;
- size_t hash2 = 1 + hash % (size - 2);
- while (table[idx].name != NULL)
- {
- idx += hash2;
- if (idx >= size)
- idx -= size;
- }
-
- table[idx].hashval = hash;
- table[idx].name = name;
- table[idx].sym = sym;
- table[idx].map = map;
- }
-
struct unique_sym_table *tab
= &GL(dl_ns)[map->l_ns]._ns_unique_sym_table;