diff mbox

remove nested functions from elf/dl-deps.c

Message ID CAGQ9bdx5q-=ovAMhhSyfnKFeNsWQ0NqMxZRAH_9f9eSmswELOA@mail.gmail.com
State New
Headers show

Commit Message

Konstantin Serebryany Oct. 1, 2014, 9:13 p.m. UTC 
Fixed all, retested. Please have another look.


On Wed, Oct 1, 2014 at 1:49 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> +static inline
>> +void preload (struct link_map *map, struct list *known, unsigned int *nlist)
>
> The return type goes on the first line.  The function name always starts
> its line.
>
> It's general policy not to use the 'inline' keyword for static functions in
> a .c file.  Unless there is a strong known reason, just let the compiler
> decide about inlining.  (This is a relatively recent policy, so you might
> find counterexamples in the code.)
>
> When a function has some parameters that are the "context" and some that
> are the specific parameters for the specific call, the style I prefer (and
> have used throughout the codebase) is to put all the "context" ones first.
>
>> +{
>> +  known[*nlist].done = 0;
>> +  known[*nlist].map = map;
>> +  known[*nlist].next = &known[*nlist + 1];
>> +
>> +  ++(*nlist);
>
> Drop superfluous parens.
>
>
> Thanks,
> Roland

Comments

Roland McGrath Oct. 1, 2014, 9:27 p.m. UTC | #1 
That's fine.  In future, always include the log entry you intend to commit
in each posting of the patch, even if it hasn't changed from the last time.
Siddhesh Poyarekar Oct. 6, 2014, 9:32 a.m. UTC | #2 
Please also update the patchwork status[1] for your patches when you
update or commit them; the MAINTAINERS page[2] describes how to get
access to the instance.

Siddhesh

[1] http://patchwork.sourceware.org/project/glibc/list/
[2] https://sourceware.org/glibc/wiki/MAINTAINERS#Becoming_a_maintainer_.28developer.29
diff mbox

Patch

diff --git a/elf/dl-deps.c b/elf/dl-deps.c
index c3b0cfc..f66b266 100644
--- a/elf/dl-deps.c
+++ b/elf/dl-deps.c
@@ -138,6 +138,19 @@  cannot load auxiliary `%s' because of empty dynamic string token "	      \
 									      \
     __result; })
 
+static void
+preload (struct list *known, unsigned int *nlist, struct link_map *map)
+{
+  known[*nlist].done = 0;
+  known[*nlist].map = map;
+  known[*nlist].next = &known[*nlist + 1];
+
+  ++*nlist;
+  /* We use `l_reserved' as a mark bit to detect objects we have
+     already put in the search list and avoid adding duplicate
+     elements later in the list.  */
+  map->l_reserved = 1;
+}
 
 void
 internal_function
@@ -155,28 +168,15 @@  _dl_map_object_deps (struct link_map *map,
   const char *errstring;
   const char *objname;
 
-  void preload (struct link_map *map)
-    {
-      known[nlist].done = 0;
-      known[nlist].map = map;
-      known[nlist].next = &known[nlist + 1];
-
-      ++nlist;
-      /* We use `l_reserved' as a mark bit to detect objects we have
-	 already put in the search list and avoid adding duplicate
-	 elements later in the list.  */
-      map->l_reserved = 1;
-    }
-
   /* No loaded object so far.  */
   nlist = 0;
 
   /* First load MAP itself.  */
-  preload (map);
+  preload (known, &nlist, map);
 
   /* Add the preloaded items after MAP but before any of its dependencies.  */
   for (i = 0; i < npreloads; ++i)
-    preload (preloads[i]);
+    preload (known, &nlist, preloads[i]);
 
   /* Terminate the lists.  */
   known[nlist - 1].next = NULL;