diff mbox

nptl: fix set-but-unused warning w/_STACK_GROWS_UP

Message ID 1438224916-27724-1-git-send-email-vapier@gentoo.org
State New
Headers show

Commit Message

Mike Frysinger July 30, 2015, 2:55 a.m. UTC 
On arches that set _STACK_GROWS_UP, the stacktop variable is declared
and set, but never actually used.  Refactor the code a bit so that the
variable is only declared/set under _STACK_GROWS_DOWN settings.

2015-07-30  Mike Frysinger  <vapier@gentoo.org>

	* nptl/allocatestack.c (allocate_stack): Move stacktop decl down to
	bottom and under _STACK_GROWS_DOWN.  Move the stacktop assignment
	in there too.
---
 nptl/allocatestack.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Roland McGrath July 30, 2015, 4:17 a.m. UTC | #1 
Looks fine for post-freeze.  I imagine it might be freeze-worthy as a bug
fix for -Werror builds on actual _STACK_GROWS_UP (is that only hppa?),
but you didn't mention that.
Mike Frysinger July 30, 2015, 5:25 a.m. UTC | #2 
On 29 Jul 2015 21:17, Roland McGrath wrote:
> Looks fine for post-freeze.  I imagine it might be freeze-worthy as a bug
> fix for -Werror builds on actual _STACK_GROWS_UP (is that only hppa?),
> but you didn't mention that.

it is a -Werror fix, but since only hppa currently has _STACK_GROWS_UP,
and hppa itself doesn't build w/out patches, i'm fine leaving this for
post freeze.
-mike
Carlos O'Donell July 31, 2015, 3:59 a.m. UTC | #3 
On 07/30/2015 01:25 AM, Mike Frysinger wrote:
> On 29 Jul 2015 21:17, Roland McGrath wrote:
>> Looks fine for post-freeze.  I imagine it might be freeze-worthy as a bug
>> fix for -Werror builds on actual _STACK_GROWS_UP (is that only hppa?),
>> but you didn't mention that.
> 
> it is a -Werror fix, but since only hppa currently has _STACK_GROWS_UP,
> and hppa itself doesn't build w/out patches, i'm fine leaving this for
> post freeze.
> -mike
> 

OK for 2.23.

Not OK for 2.22, I don't want this to inadvertently break anything.

c.
Mike Frysinger Aug. 5, 2015, 8:40 a.m. UTC | #4 
On 30 Jul 2015 23:59, Carlos O'Donell wrote:
> On 07/30/2015 01:25 AM, Mike Frysinger wrote:
> > On 29 Jul 2015 21:17, Roland McGrath wrote:
> >> Looks fine for post-freeze.  I imagine it might be freeze-worthy as a bug
> >> fix for -Werror builds on actual _STACK_GROWS_UP (is that only hppa?),
> >> but you didn't mention that.
> > 
> > it is a -Werror fix, but since only hppa currently has _STACK_GROWS_UP,
> > and hppa itself doesn't build w/out patches, i'm fine leaving this for
> > post freeze.
> 
> OK for 2.23.

pushed thanks
-mike
diff mbox

Patch

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index c56a4df..753da61 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -353,7 +353,6 @@  allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
   struct pthread *pd;
   size_t size;
   size_t pagesize_m1 = __getpagesize () - 1;
-  void *stacktop;
 
   assert (powerof2 (pagesize_m1 + 1));
   assert (TCB_ALIGNMENT >= STACK_ALIGN);
@@ -717,19 +716,23 @@  allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
   /* We place the thread descriptor at the end of the stack.  */
   *pdp = pd;
 
-#if TLS_TCB_AT_TP
+#if _STACK_GROWS_DOWN
+  void *stacktop;
+
+# if TLS_TCB_AT_TP
   /* The stack begins before the TCB and the static TLS block.  */
   stacktop = ((char *) (pd + 1) - __static_tls_size);
-#elif TLS_DTV_AT_TP
+# elif TLS_DTV_AT_TP
   stacktop = (char *) (pd - 1);
-#endif
+# endif
 
-#ifdef NEED_SEPARATE_REGISTER_STACK
+# ifdef NEED_SEPARATE_REGISTER_STACK
   *stack = pd->stackblock;
   *stacksize = stacktop - *stack;
-#elif _STACK_GROWS_DOWN
+# else
   *stack = stacktop;
-#elif _STACK_GROWS_UP
+# endif
+#else
   *stack = pd->stackblock;
   assert (*stack > 0);
 #endif