| Message ID | 20140605090455.GB9145@spoyarek.pnq.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 5, 2014 at 1:04 PM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote: > On Wed, Jun 04, 2014 at 10:16:23AM -0700, Roland McGrath wrote: >> That is fine, though it wouldn't hurt to improve the commentary and use >> bool while you're there. > > I wonder if it would be better to just inline this function: This seems to be equally ok. > > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c > index 1e22f7d..b779529 100644 > --- a/nptl/allocatestack.c > +++ b/nptl/allocatestack.c > @@ -830,26 +830,23 @@ __reclaim_stacks (void) > > if (add_p) > { > - /* We always add at the beginning of the list. So in this > - case we only need to check the beginning of these lists. */ > - int check_list (list_t *l) > - { > - if (l->next->prev != l) > - { > - assert (l->next->prev == elem); > - > - elem->next = l->next; > - elem->prev = l; > - l->next = elem; > - > - return 1; > - } > - > - return 0; > - } > - > - if (check_list (&stack_used) == 0) > - (void) check_list (&stack_cache); > + /* We always add at the beginning of the list. So in this case we > + only need to check the beginning of these lists to see if the > + pointers at the head of the list are inconsistent. */ > + list_t *l = NULL; > + > + if (stack_used.next->prev != &stack_used) > + l = &stack_used; > + else if (stack_cache.next->prev != &stack_cache) > + l = &stack_cache; > + > + if (l) > + { > + assert (l->next->prev == elem); > + elem->next = l->next; > + elem->prev = l; > + l->next = elem; > + } > } > else > {
Siddhesh Poyarekar <siddhesh@redhat.com> writes:
> I wonder if it would be better to just inline this function:
This is much better.
Andreas.
> + /* We always add at the beginning of the list. So in this case we > + only need to check the beginning of these lists to see if the > + pointers at the head of the list are inconsistent. */ > + list_t *l = NULL; > + > + if (stack_used.next->prev != &stack_used) > + l = &stack_used; > + else if (stack_cache.next->prev != &stack_cache) > + l = &stack_cache; > + > + if (l) No implicit Boolean coercion. Otherwise this is fine.
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 1e22f7d..b779529 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -830,26 +830,23 @@ __reclaim_stacks (void) if (add_p) { - /* We always add at the beginning of the list. So in this - case we only need to check the beginning of these lists. */ - int check_list (list_t *l) - { - if (l->next->prev != l) - { - assert (l->next->prev == elem); - - elem->next = l->next; - elem->prev = l; - l->next = elem; - - return 1; - } - - return 0; - } - - if (check_list (&stack_used) == 0) - (void) check_list (&stack_cache); + /* We always add at the beginning of the list. So in this case we + only need to check the beginning of these lists to see if the + pointers at the head of the list are inconsistent. */ + list_t *l = NULL; + + if (stack_used.next->prev != &stack_used) + l = &stack_used; + else if (stack_cache.next->prev != &stack_cache) + l = &stack_cache; + + if (l) + { + assert (l->next->prev == elem); + elem->next = l->next; + elem->prev = l; + l->next = elem; + } } else {