This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] remove one nested function from nptl/allocatestack.c
- From: Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Thu, 5 Jun 2014 13:10:06 +0400
- Subject: Re: [PATCH] remove one nested function from nptl/allocatestack.c
- Authentication-results: sourceware.org; auth=none
- References: <CAGQ9bdw9_bgxZ3UOMNnnMC7EXtyUbBs5AkHvEF3jT0uB3GhaSg at mail dot gmail dot com> <20140604171623 dot 362652C39B4 at topped-with-meat dot com> <20140605090455 dot GB9145 at spoyarek dot pnq dot redhat dot com>
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
> {