Fix build warning in pthread_rwlock_*

Message ID 20140619165137.GR7238@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar June 19, 2014, 4:51 p.m. UTC
  Hi,

The first argument of elision_adapt and that of ELISION_*LOCK have
different signs since __elision_rwcount is singned char and the
argument of elision_adapt is uint8_t.  Modified elision_adapt to
accept int8_t instead of uint8_t.

Siddhesh

	* sysdeps/x86/nptl/elide.h (elision_adapt): Make first
	argument type int8_t.
  

Comments

Siddhesh Poyarekar June 24, 2014, 9:03 a.m. UTC | #1
Ping!

On Thu, Jun 19, 2014 at 10:21:38PM +0530, Siddhesh Poyarekar wrote:
> Hi,
> 
> The first argument of elision_adapt and that of ELISION_*LOCK have
> different signs since __elision_rwcount is singned char and the
> argument of elision_adapt is uint8_t.  Modified elision_adapt to
> accept int8_t instead of uint8_t.
> 
> Siddhesh
> 
> 	* sysdeps/x86/nptl/elide.h (elision_adapt): Make first
> 	argument type int8_t.
> 
> diff --git a/sysdeps/x86/nptl/elide.h b/sysdeps/x86/nptl/elide.h
> index 19f27e5..3979146 100644
> --- a/sysdeps/x86/nptl/elide.h
> +++ b/sysdeps/x86/nptl/elide.h
> @@ -26,7 +26,7 @@
>  /* Adapt elision with ADAPT_COUNT and STATUS and decide retries.  */
>  
>  static inline bool
> -elision_adapt(uint8_t *adapt_count, unsigned int status)
> +elision_adapt(int8_t *adapt_count, unsigned int status)
>  {
>    if (status & _XABORT_RETRY)
>      return false;
  
Will Newton June 24, 2014, 9:13 a.m. UTC | #2
On 19 June 2014 17:51, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
> Hi,
>
> The first argument of elision_adapt and that of ELISION_*LOCK have
> different signs since __elision_rwcount is singned char and the
> argument of elision_adapt is uint8_t.  Modified elision_adapt to
> accept int8_t instead of uint8_t.
>
> Siddhesh
>
>         * sysdeps/x86/nptl/elide.h (elision_adapt): Make first
>         argument type int8_t.

This looks ok to me, although I don't have the hardware to test.

> diff --git a/sysdeps/x86/nptl/elide.h b/sysdeps/x86/nptl/elide.h
> index 19f27e5..3979146 100644
> --- a/sysdeps/x86/nptl/elide.h
> +++ b/sysdeps/x86/nptl/elide.h
> @@ -26,7 +26,7 @@
>  /* Adapt elision with ADAPT_COUNT and STATUS and decide retries.  */
>
>  static inline bool
> -elision_adapt(uint8_t *adapt_count, unsigned int status)
> +elision_adapt(int8_t *adapt_count, unsigned int status)
>  {
>    if (status & _XABORT_RETRY)
>      return false;
  
Andreas Schwab June 24, 2014, 9:54 a.m. UTC | #3
Siddhesh Poyarekar <siddhesh@redhat.com> writes:

> The first argument of elision_adapt and that of ELISION_*LOCK have
> different signs since __elision_rwcount is singned char and the

__elision_rwcount doesn't exist.

> argument of elision_adapt is uint8_t.  Modified elision_adapt to
> accept int8_t instead of uint8_t.

Make it signed char.

Andreas.
  
Roland McGrath June 24, 2014, 8:18 p.m. UTC | #4
Yes, please.  I've also noticed that the elision code seems to have a lot
of little style violations (missing space before paren and the like) that
people didn't catch in review.  It could all use going over for clean
source and clean builds.
  

Patch

diff --git a/sysdeps/x86/nptl/elide.h b/sysdeps/x86/nptl/elide.h
index 19f27e5..3979146 100644
--- a/sysdeps/x86/nptl/elide.h
+++ b/sysdeps/x86/nptl/elide.h
@@ -26,7 +26,7 @@ 
 /* Adapt elision with ADAPT_COUNT and STATUS and decide retries.  */
 
 static inline bool
-elision_adapt(uint8_t *adapt_count, unsigned int status)
+elision_adapt(int8_t *adapt_count, unsigned int status)
 {
   if (status & _XABORT_RETRY)
     return false;