On 10/07/2013 03:10 PM, Greg KH wrote:
On Mon, Oct 07, 2013 at 12:43:41PM -0500, Larry Finger wrote:
At present, if one wants to free the memory allocation used for
a dev->kobj name, it is necessary to go quite deeply into the structure.
Why would you ever want to do this?
To avoid this much dependence on the structure details in driver
code, a new inline routine is created.
Signed-off-by: Larry Finger <larry.fin...@lwfinger.net>
---
Index: wireless-testing-save/include/linux/device.h
===================================================================
--- wireless-testing-save.orig/include/linux/device.h
+++ wireless-testing-save/include/linux/device.h
@@ -27,6 +27,7 @@
#include <linux/ratelimit.h>
#include <linux/uidgid.h>
#include <asm/device.h>
+#include <linux/slab.h>
struct device;
struct device_private;
@@ -789,6 +790,11 @@ static inline const char *dev_name(const
return kobject_name(&dev->kobj);
}
+static inline void dev_free_name(struct device *dev)
+{
+ kfree(dev->kobj.name);
+}
Please show how you would use this function, I can't add functions that
no one calls.
And given that this type of thing hasn't been needed before, I'm
thinking that it still isn't needed :)
In the thread at
http://lkml.indiana.edu/hypermail/linux/kernel/1310.0/02008.html, I reported a
leak of kobj->name in the error path of the memstick driver. My solution was to
free it in the error path by using
kfree(card->dev.kobj.name);
Catalin Marinas responded with "It looks weird to go into dev.kobj internals
here for freeing the name. There is also memstick_free_card() which doesn't seem
to do anything about the name freeing."
Later in the thread, he agreed that having a new function sounded like a good
idea. I should have submitted the second patch using the new function as follows:
Index: wireless-testing-save/drivers/memstick/core/memstick.c
===================================================================
--- wireless-testing-save.orig/drivers/memstick/core/memstick.c
+++ wireless-testing-save/drivers/memstick/core/memstick.c
@@ -195,6 +195,7 @@ static void memstick_free_card(struct de
{
struct memstick_dev *card = container_of(dev, struct memstick_dev,
dev);
+ dev_free_name(&card->dev);
kfree(card);
}
@@ -415,6 +416,7 @@ static struct memstick_dev *memstick_all
return card;
err_out:
host->card = old_card;
+ dev_free_name(&card->dev);
kfree(card);
return NULL;
}
These changes clear up the memory leak that started the whole subject. The
difference would be between
kfree(card->dev.kobj.name);
and
dev_free_name(&card->dev);
I have not looked at every line in the kernel containing both "kfree" and "name"
to see how many other places that the new routine could be used. I'm not even
sure how other drivers free the space allocated for a name.
Larry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
