| Message ID | CAGQ9bdwXd0AVUKRwtosJw+d5QujmqNspeVHG+2N9O+o1_F-KqQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c index 7c32830..5d3c384 100644 --- a/elf/dl-lookup.c +++ b/elf/dl-lookup.c @@ -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;
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.