[alsa-devel] [PATCH 0/4] Fixes and cleanups for the recent procfs changes
Hi,
here is a series of patches to fix and clean up the recent changes for ALSA core procfs support code.
Takashi
===
Takashi Iwai (4): ALSA: info: Fix leaks of child entries at snd_info_free_entry() ALSA: info: Register proc entries recursively, too ALSA: info: Move list addition to snd_info_create_entry() ALSA: info: Drop kerneldoc comment from snd_info_create_entry()
sound/core/info.c | 62 +++++++++++++++++++++++++++++++++++++++++-------------- sound/core/init.c | 18 ++++++---------- 2 files changed, 52 insertions(+), 28 deletions(-)
snd_info_free_entry() releases the all children nodes as well, but due to the wrong timing of releasing the link, the children nodes may be disconnected but left unreleased. This patch fixes it by moving the link free at the right position. Also it eases list_for_each_entry() without _safe option in snd_info_disconnect() because it no longer frees the children nodes there.
Fixes: c560a6797e3b ('ALSA: core: Remove child proc file elements recursively') Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/info.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/core/info.c b/sound/core/info.c index c8a413d6cc9b..566279374683 100644 --- a/sound/core/info.c +++ b/sound/core/info.c @@ -738,13 +738,12 @@ EXPORT_SYMBOL(snd_info_create_card_entry);
static void snd_info_disconnect(struct snd_info_entry *entry) { - struct snd_info_entry *p, *n; + struct snd_info_entry *p;
if (!entry->p) return; - list_for_each_entry_safe(p, n, &entry->children, list) + list_for_each_entry(p, &entry->children, list) snd_info_disconnect(p); - list_del_init(&entry->list); proc_remove(entry->p); entry->p = NULL; } @@ -771,6 +770,7 @@ void snd_info_free_entry(struct snd_info_entry * entry) list_for_each_entry_safe(p, n, &entry->children, list) snd_info_free_entry(p);
+ list_del(&entry->list); kfree(entry->name); if (entry->private_free) entry->private_free(entry);
The commit [c560a6797e3b: ALSA: core: Remove child proc file elements recursively] converted snd_card_proc_new() with the normal snd_info_*() call and removed snd_device chain for such info entries. However, it misses one point: the creation of the proc entry was managed by snd_device chain in the former code, and now it's also gone, which results in no proc files creation at all. Mea culpa.
This patch makes snd_info_card_register() creating the all pending child proc entries in a shot. Also, since snd_card_register() might be called multiple times, this function is also changed to be callable multiple times.
Along with the changes above, now the linked list of snd_info_entry is added at creation time instead of snd_info_register() for keeping eyes of pending info entries.
Fixes: c560a6797e3b ('ALSA: core: Remove child proc file elements recursively') Reported-by: "Lu, Han" han.lu@intel.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/info.c | 37 ++++++++++++++++++++++++++++++++++--- sound/core/init.c | 18 ++++++------------ 2 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/sound/core/info.c b/sound/core/info.c index 566279374683..13b174464cc8 100644 --- a/sound/core/info.c +++ b/sound/core/info.c @@ -515,22 +515,51 @@ int snd_info_card_create(struct snd_card *card) return 0; }
+/* register all pending info entries */ +static int snd_info_register_recursive(struct snd_info_entry *entry) +{ + struct snd_info_entry *p; + int err; + + if (!entry->p) { + err = snd_info_register(entry); + if (err < 0) + return err; + } + + list_for_each_entry(p, &entry->children, list) { + err = snd_info_register_recursive(p); + if (err < 0) + return err; + } + + return 0; +} + /* * register the card proc file * called from init.c + * can be called multiple times for reinitialization */ int snd_info_card_register(struct snd_card *card) { struct proc_dir_entry *p; + int err;
if (snd_BUG_ON(!card)) return -ENXIO;
+ err = snd_info_register_recursive(card->proc_root); + if (err < 0) + return err; + if (!strcmp(card->id, card->proc_root->name)) return 0;
+ if (card->proc_root_link) + return 0; p = proc_symlink(card->id, snd_proc_root->p, card->proc_root->name); - if (p == NULL) + if (!p) return -ENOMEM; card->proc_root_link = p; return 0; @@ -705,6 +734,8 @@ struct snd_info_entry *snd_info_create_module_entry(struct module * module, if (entry) { entry->module = module; entry->parent = parent; + if (parent) + list_add_tail(&entry->list, &parent->children); } return entry; } @@ -730,6 +761,8 @@ struct snd_info_entry *snd_info_create_card_entry(struct snd_card *card, entry->module = card->module; entry->card = card; entry->parent = parent; + if (parent) + list_add_tail(&entry->list, &parent->children); } return entry; } @@ -816,8 +849,6 @@ int snd_info_register(struct snd_info_entry * entry) proc_set_size(p, entry->size); } entry->p = p; - if (entry->parent) - list_add_tail(&entry->list, &entry->parent->children); mutex_unlock(&info_mutex); return 0; } diff --git a/sound/core/init.c b/sound/core/init.c index 769a783757ff..f8abd2d8144e 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -107,26 +107,20 @@ static void snd_card_id_read(struct snd_info_entry *entry, snd_iprintf(buffer, "%s\n", entry->card->id); }
-static inline int init_info_for_card(struct snd_card *card) +static int init_info_for_card(struct snd_card *card) { int err; struct snd_info_entry *entry;
- if ((err = snd_info_card_register(card)) < 0) { - dev_dbg(card->dev, "unable to create card info\n"); - return err; - } - if ((entry = snd_info_create_card_entry(card, "id", card->proc_root)) == NULL) { + entry = snd_info_create_card_entry(card, "id", card->proc_root); + if (!entry) { dev_dbg(card->dev, "unable to create card entry\n"); return err; } entry->c.text.read = snd_card_id_read; - if (snd_info_register(entry) < 0) { - snd_info_free_entry(entry); - entry = NULL; - } card->proc_id = entry; - return 0; + + return snd_info_card_register(card); } #else /* !CONFIG_PROC_FS */ #define init_info_for_card(card) @@ -756,7 +750,7 @@ int snd_card_register(struct snd_card *card) if (snd_cards[card->number]) { /* already registered */ mutex_unlock(&snd_card_mutex); - return 0; + return snd_info_card_register(card); /* register pending info */ } if (*card->id) { /* make a unique id name from the given string */
Just a minor refactoring, no functional changes.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/info.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/sound/core/info.c b/sound/core/info.c index 13b174464cc8..76cdf1d21f17 100644 --- a/sound/core/info.c +++ b/sound/core/info.c @@ -455,11 +455,12 @@ static struct snd_info_entry *create_subdir(struct module *mod, return entry; }
-static struct snd_info_entry *snd_info_create_entry(const char *name); +static struct snd_info_entry * +snd_info_create_entry(const char *name, struct snd_info_entry *parent);
int __init snd_info_init(void) { - snd_proc_root = snd_info_create_entry("asound"); + snd_proc_root = snd_info_create_entry("asound", NULL); if (!snd_proc_root) return -ENOMEM; snd_proc_root->mode = S_IFDIR | S_IRUGO | S_IXUGO; @@ -688,6 +689,7 @@ EXPORT_SYMBOL(snd_info_get_str); /** * snd_info_create_entry - create an info entry * @name: the proc file name + * @parent: the parent directory * * Creates an info entry with the given file name and initializes as * the default state. @@ -697,7 +699,8 @@ EXPORT_SYMBOL(snd_info_get_str); * * Return: The pointer of the new instance, or %NULL on failure. */ -static struct snd_info_entry *snd_info_create_entry(const char *name) +static struct snd_info_entry * +snd_info_create_entry(const char *name, struct snd_info_entry *parent) { struct snd_info_entry *entry; entry = kzalloc(sizeof(*entry), GFP_KERNEL); @@ -713,6 +716,9 @@ static struct snd_info_entry *snd_info_create_entry(const char *name) mutex_init(&entry->access); INIT_LIST_HEAD(&entry->children); INIT_LIST_HEAD(&entry->list); + entry->parent = parent; + if (parent) + list_add_tail(&entry->list, &parent->children); return entry; }
@@ -730,13 +736,9 @@ struct snd_info_entry *snd_info_create_module_entry(struct module * module, const char *name, struct snd_info_entry *parent) { - struct snd_info_entry *entry = snd_info_create_entry(name); - if (entry) { + struct snd_info_entry *entry = snd_info_create_entry(name, parent); + if (entry) entry->module = module; - entry->parent = parent; - if (parent) - list_add_tail(&entry->list, &parent->children); - } return entry; }
@@ -756,13 +758,10 @@ struct snd_info_entry *snd_info_create_card_entry(struct snd_card *card, const char *name, struct snd_info_entry * parent) { - struct snd_info_entry *entry = snd_info_create_entry(name); + struct snd_info_entry *entry = snd_info_create_entry(name, parent); if (entry) { entry->module = card->module; entry->card = card; - entry->parent = parent; - if (parent) - list_add_tail(&entry->list, &parent->children); } return entry; }
It's no longer a part of API but merely a local function.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/info.c b/sound/core/info.c index 76cdf1d21f17..895362a696c9 100644 --- a/sound/core/info.c +++ b/sound/core/info.c @@ -686,7 +686,7 @@ const char *snd_info_get_str(char *dest, const char *src, int len)
EXPORT_SYMBOL(snd_info_get_str);
-/** +/* * snd_info_create_entry - create an info entry * @name: the proc file name * @parent: the parent directory
participants (1)
-
Takashi Iwai