[sheepdog] [PATCH v9 3/4] lib: correct a way of freeing sockfd_cache_entry

[Hitoshi Mitake]

At Wed, 31 Jul 2013 17:21:56 +0800,
Liu Yuan wrote:
> 
> On Wed, Jul 31, 2013 at 05:44:44PM +0900, Hitoshi Mitake wrote:
> > Previous implementation of the sockfd caching mechanism has two memory
> > leaks. The leak is caused when sockfd_cache_destroy() and
> > sockfd_cache_add_nolock() try to free a sockfd cache entry. They
> > forget to free entry->fds.
> > 
> > This patch adds a new function, free_cache_entry() for safe freeing of
> > sockfd cache entries. And let the above two functions to use it.
> > 
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> > v9:
> >  - remove an unnecessary check in free_cache_entry()
> > 
> >  lib/sockfd_cache.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/sockfd_cache.c b/lib/sockfd_cache.c
> > index 64c44d8..b0ae596 100644
> > --- a/lib/sockfd_cache.c
> > +++ b/lib/sockfd_cache.c
> > @@ -179,6 +179,12 @@ static inline void destroy_all_slots(struct sockfd_cache_entry *entry)
> >  			close(entry->fds[i].fd);
> >  }
> >  
> > +static void free_cache_entry(struct sockfd_cache_entry *entry)
> > +{
> > +	free(entry->fds);
> > +	free(entry);
> > +}
> > +
> >  /*
> >   * Destroy all the Cached FDs of the node
> >   *
> > @@ -206,7 +212,7 @@ static bool sockfd_cache_destroy(const struct node_id *nid)
> >  	pthread_rwlock_unlock(&sockfd_cache.lock);
> >  
> >  	destroy_all_slots(entry);
> > -	free(entry);
> > +	free_cache_entry(entry);
> >  
> >  	return true;
> >  false_out:
> > @@ -225,7 +231,7 @@ static void sockfd_cache_add_nolock(const struct node_id *nid)
> >  
> >  	memcpy(&new->nid, nid, sizeof(struct node_id));
> >  	if (sockfd_cache_insert(new)) {
> > -		free(new);
> > +		free_cache_entry(new);
> >  		return;
> >  	}
> >  	sockfd_cache.count++;
> > -- 
> > 1.7.10.4
> > 
> > -- 
> > sheepdog mailing list
> > sheepdog at lists.wpkg.org
> > http://lists.wpkg.org/mailman/listinfo/sheepdog
> 
> sockfd_cache_add() should call free_cache_entry() too.

Thanks for your pointing, I'll fix it in v10.

Thanks,
Hitoshi