diff mbox

[roland/cancelhandling] NPTL: Give cancelhandling fields type unsigned int.

Message ID 20150625000729.9C7912C3B00@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath June 25, 2015, 12:07 a.m. UTC 
This perturbed code generation for pthread_detach.c a bit more than I would
have thought (on x86_64, i686, and arm), and I can't see any reason for
that.  But I think the semantics of the generated code are identical.  The
only other change to generated code is for nptl-init.c, where the compiler
reversed the order of operands in a comparison instruction--but one where
only eq/ne is ever tested, so it's trivially equivalent.

I wouldn't mind another set of eyes on the code generation changes to
verify they are indeed wholly harmless.

But if nobody objects soon, I'll commit this.


Thanks,
Roland

	* nptl/descr.h (struct pthread): Change type of fields cancelhandling
	and parent_cancelhandling from 'int' to 'unsigned int'.

Comments

Torvald Riegel June 30, 2015, 5:11 p.m. UTC | #1 
On Wed, 2015-06-24 at 17:07 -0700, Roland McGrath wrote:
> This perturbed code generation for pthread_detach.c a bit more than I would
> have thought (on x86_64, i686, and arm), and I can't see any reason for
> that.  But I think the semantics of the generated code are identical.  The
> only other change to generated code is for nptl-init.c, where the compiler
> reversed the order of operands in a comparison instruction--but one where
> only eq/ne is ever tested, so it's trivially equivalent.
> 
> I wouldn't mind another set of eyes on the code generation changes to
> verify they are indeed wholly harmless.

What about the loads from cancelhandling that assign to an int variable
in these files:
nptl/cleanup_defer_compat.c
nptl/allocatestack.c
nptl/pthread_setcanceltype.c
nptl/cancellation.c
nptl/cleanup_defer.c
...

Shouldn't those variables be changed to unsigned int too?

All (or almost all) of those accesses should use atomics.  It may be
better to keep the casts in the futex_* calls for now and clean this up
in one chunk, using atomics and unsigned int everywhere.
diff mbox

Patch

diff --git a/nptl/descr.h b/nptl/descr.h
index 5bd1282..818e11a 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -256,7 +256,7 @@  struct pthread
 #define HAVE_CLEANUP_JMP_BUF
 
   /* Flags determining processing of cancellation.  */
-  int cancelhandling;
+  unsigned int cancelhandling;
   /* Bit set if cancellation is disabled.  */
 #define CANCELSTATE_BIT		0
 #define CANCELSTATE_BITMASK	(0x01 << CANCELSTATE_BIT)
@@ -322,7 +322,7 @@  struct pthread
 
   /* The parent's cancel handling at the time of the pthread_create
      call.  This might be needed to undo the effects of a cancellation.  */
-  int parent_cancelhandling;
+  unsigned int parent_cancelhandling;
 
   /* Lock to synchronize access to the descriptor.  */
   int lock;