diff mbox

[2/2] Initialize all of datahead structure in nscd [BZ #16791]

Message ID 20140402094011.GD23931@spoyarek.pnq.redhat.com
State New
Headers show

Commit Message

Siddhesh Poyarekar April 2, 2014, 9:40 a.m. UTC 
On Wed, Apr 02, 2014 at 05:33:44AM -0400, Mike Frysinger wrote:
> On Wed 02 Apr 2014 10:23:55 Siddhesh Poyarekar wrote:
> > On Tue, Apr 01, 2014 at 11:26:16AM -0700, Roland McGrath wrote:
> > > If you bzero before explicitly filling fields, does the compiler optimize
> > > to only zeroing the one unused field?  If so, that seems more futureproof.
> > 
> > It doesn't quite, but (now that I checked) it really only results in a
> > couple of additional instructions (as opposed to a function call that
> > I had implicitly assumed), so I'll add it anyway if you think it is
> > OK.  This obviously also needs the previous patch that consolidates
> > the head initialization code.
> 
> as a general thing, can we not use bzero ?  like ever ?  memset should be 
> exactly the same.

Yeah, sorry.  Here:

Comments

Mike Frysinger April 2, 2014, 9:42 a.m. UTC | #1 
On Wed 02 Apr 2014 15:10:12 Siddhesh Poyarekar wrote:
> @@ -253,18 +257,21 @@ static inline time_t
>  datahead_init_pos (struct datahead *head, nscd_ssize_t allocsize,
>  		   nscd_ssize_t recsize, uint8_t nreloads, uint32_t ttl)
>  {
> +  time_t ret = datahead_init_common (head, allocsize, recsize, ttl);
>    head->notfound = false;

doesn't GNU style say there should be a new line between decls and code ?  or 
do we not care about that rule in glibc ?

>    head->nreloads = nreloads;
> -  return datahead_init_common (head, allocsize, recsize, ttl);
> +  return ret;

same for return value.

>  static inline time_t
>  datahead_init_neg (struct datahead *head, nscd_ssize_t allocsize,
>  		   nscd_ssize_t recsize, uint32_t ttl)
>  {
> +  time_t ret = datahead_init_common (head, allocsize, recsize, ttl);
> +  /* We don't need to touch nreloads here since it is set to our desired
> value
> +     (0) when we bzero the structure.  */

bzero->clear
-mike
diff mbox

Patch

diff --git a/nscd/nscd-client.h b/nscd/nscd-client.h
index c069bf6..954fe99 100644
--- a/nscd/nscd-client.h
+++ b/nscd/nscd-client.h
@@ -240,6 +240,10 @@  static inline time_t
 datahead_init_common (struct datahead *head, nscd_ssize_t allocsize,
 		      nscd_ssize_t recsize, uint32_t ttl)
 {
+  /* Initialize so that we don't write out junk in uninitialized data to the
+     cache.  */
+  memset (head, 0, sizeof (*head));
+
   head->allocsize = allocsize;
   head->recsize = recsize;
   head->usable = true;
@@ -253,18 +257,21 @@  static inline time_t
 datahead_init_pos (struct datahead *head, nscd_ssize_t allocsize,
 		   nscd_ssize_t recsize, uint8_t nreloads, uint32_t ttl)
 {
+  time_t ret = datahead_init_common (head, allocsize, recsize, ttl);
   head->notfound = false;
   head->nreloads = nreloads;
-  return datahead_init_common (head, allocsize, recsize, ttl);
+  return ret;
 }
 
 static inline time_t
 datahead_init_neg (struct datahead *head, nscd_ssize_t allocsize,
 		   nscd_ssize_t recsize, uint32_t ttl)
 {
+  time_t ret = datahead_init_common (head, allocsize, recsize, ttl);
+  /* We don't need to touch nreloads here since it is set to our desired value
+     (0) when we bzero the structure.  */
   head->notfound = true;
-  head->nreloads = 0;
-  return datahead_init_common (head, allocsize, recsize, ttl);
+  return ret;
 }
 
 /* Structure for one hash table entry.  */