Re: [v2] afs: Fix memory leak in afs_put_sysnames()

[David Howells]

Markus Elfring <Markus.Elfring@xxxxxx> wrote:

> > 	Fix afs_put_sysnames() to actually free the specified afs_sysnames
> > 	object after its reference count has been decreased to zero and its
> > 	contents have been released.
> 
> * How do you think about to omit the word "Fix" because of the provided tag?

Quite often I might put introductory paragraphs before that, so I prefer to
begin the paragraph that states a fix with that verb.  There may also be
auxiliary changes associated with it that aren't directly fixes but need to be
made because of the fix change.

> * Is freeing and releasing an item a duplicate operation anyhow?

You're missing the point.  afs_put_sysnames() does release the things the
object points to (ie. the content), but not the object itself.

> >> Will it matter to mention the size of the data structure "afs_sysnames"?
> >
> > Why is it necessary to do so?
> 
> I suggest to express the impact of the missed function call "kfree".

I would hope that anyone reading the patch could work the impact out for
themselves.  Just specifying the size of a struct isn't all that useful - it
may be wildly variable by arch (eg. 32/64) and config option (eg. lockdep)
anyway.  Add to that rounding and packing details from the memory subsys,
along with the pinning effect of something you can't get rid of.

Of more use would be specifying the frequency or likelyhood of such a leak but
unless it's especially high, it's probably not worth mentioning.

David