[V2] Initialize pad outside the conditional to prevent uninitialized data warnings.

Message ID CAOraFgDHcsCAOCrySVPmrhX-87PFZ1Zrqg3hQoLioNSnmhr5Cw@mail.gmail.com
State New, archived
Headers

Commit Message

Patsy Franklin June 26, 2018, 3:13 p.m. UTC
  Updated patch attached.
  

Comments

Carlos O'Donell June 26, 2018, 5:08 p.m. UTC | #1
On 06/26/2018 11:13 AM, Patsy Franklin wrote:
> Updated patch attached.

This version looks good to me.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> commit afcfe098889bb3e5e04c662e9047f20864d55f8e
> Author: Patsy Franklin <pfrankli@redhat.com>
> Date:   Tue Jun 26 10:35:03 2018 -0400
> 
>     In sem_open.c,  pad was not initialized when __HAVE_64B_ATOMICS was
>     true.  On some arches this caused valgrind to warn about uninitialized
>     bytes when the struct was written to the file system.

OK, commit message resolves Florian's question about why this raises an error.
It's an error because we write uninitialized bytes to disk which can be seen
by a reader.

>     
>     This patch moves the initialization of pad outside of the
>     conditional.
> 
> diff --git a/ChangeLog b/ChangeLog
> index eaee727..83539e3 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-06-26  Patsy Franklin  <pfrankli@redhat.com>
> +
> +	* nptl/sem_open.c [!__HAVE_64B_ATOMICS] (sem_open): Don't update pad.
> +	(sem_open): Set sem.newsem.pad to zero for valgrind.
> +

OK.

>  2018-06-26  Florian Weimer  <fweimer@redhat.com>
>  
>  	Run thread shutdown functions in an explicit order.
> diff --git a/nptl/sem_open.c b/nptl/sem_open.c
> index 1d7f142..c5389f6 100644
> --- a/nptl/sem_open.c
> +++ b/nptl/sem_open.c
> @@ -215,10 +215,11 @@ sem_open (const char *name, int oflag, ...)
>        sem.newsem.data = value;
>  #else
>        sem.newsem.value = value << SEM_VALUE_SHIFT;
> -      /* pad is used as a mutex on pre-v9 sparc and ignored otherwise.  */
> -      sem.newsem.pad = 0;
>        sem.newsem.nwaiters = 0;
>  #endif
> +      /* pad is used as a mutex on pre-v9 sparc and ignored otherwise.  */
> +      sem.newsem.pad = 0;

OK.

> +
>        /* This always is a shared semaphore.  */
>        sem.newsem.private = FUTEX_SHARED;
>  

Cheers,
Carlos.
  

Patch

commit afcfe098889bb3e5e04c662e9047f20864d55f8e
Author: Patsy Franklin <pfrankli@redhat.com>
Date:   Tue Jun 26 10:35:03 2018 -0400

    In sem_open.c,  pad was not initialized when __HAVE_64B_ATOMICS was
    true.  On some arches this caused valgrind to warn about uninitialized
    bytes when the struct was written to the file system.
    
    This patch moves the initialization of pad outside of the
    conditional.

diff --git a/ChangeLog b/ChangeLog
index eaee727..83539e3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-06-26  Patsy Franklin  <pfrankli@redhat.com>
+
+	* nptl/sem_open.c [!__HAVE_64B_ATOMICS] (sem_open): Don't update pad.
+	(sem_open): Set sem.newsem.pad to zero for valgrind.
+
 2018-06-26  Florian Weimer  <fweimer@redhat.com>
 
 	Run thread shutdown functions in an explicit order.
diff --git a/nptl/sem_open.c b/nptl/sem_open.c
index 1d7f142..c5389f6 100644
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -215,10 +215,11 @@  sem_open (const char *name, int oflag, ...)
       sem.newsem.data = value;
 #else
       sem.newsem.value = value << SEM_VALUE_SHIFT;
-      /* pad is used as a mutex on pre-v9 sparc and ignored otherwise.  */
-      sem.newsem.pad = 0;
       sem.newsem.nwaiters = 0;
 #endif
+      /* pad is used as a mutex on pre-v9 sparc and ignored otherwise.  */
+      sem.newsem.pad = 0;
+
       /* This always is a shared semaphore.  */
       sem.newsem.private = FUTEX_SHARED;