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

Hitoshi Mitake mitake.hitoshi at gmail.com
Wed Jul 31 09:34:32 CEST 2013


At Wed, 31 Jul 2013 15:29:45 +0800,
Liu Yuan wrote:
> 
> On Wed, Jul 31, 2013 at 04:11:48PM +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>
> > ---
> >  lib/sockfd_cache.c |   11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/sockfd_cache.c b/lib/sockfd_cache.c
> > index 98cea94..9a87ed9 100644
> > --- a/lib/sockfd_cache.c
> > +++ b/lib/sockfd_cache.c
> > @@ -179,6 +179,13 @@ 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)
> > +{
> > +	if (entry->fds)
> > +		free(entry->fds);
> > +	free(entry);
> > +}
> 
> this is just a internal function and we know ->fds is always allocated and
> even if ->fds == NULL, free(NULL) is valid. so if check is unnecessary. 

I thought that I did remove the branch before posting... I'll fix in v9.

Thanks,
Hitoshi



More information about the sheepdog mailing list