[PATCH 0/8] Proc file cleanups

[PATCH 0/8] Proc file cleanups

[Takashi Iwai]

Hi,

here is a series of patches for cleaning up the ALSA proc file
management code.  The biggest change is the use of seq_file for text
reads, but all are internal and the API is kept intact.

The rest are mostly small cleanups and refactoring.

The patches are found in topic/proc branch of sound git tree.


Takashi

===

Takashi Iwai (8):
  ALSA: core: Use seq_file for text proc file reads
  ALSA: core: Fix possible memory leaks at error path in info.c
  ALSA: core: Remove child proc file elements recursively
  ALSA: core: Manage asound root directory with snd_info_entry
  ALSA: core: Remove superfluous exit calls for proc entries
  ALSA: core: Don't ignore errors at creating proc files
  ALSA: core: Build conditionally and remove superfluous ifdefs
  ALSA: core: Clean up OSS proc file management

 include/sound/core.h      |   4 -
 include/sound/info.h      |  29 +-
 sound/core/Makefile       |   8 +-
 sound/core/info.c         | 779 ++++++++++++++++++----------------------------
 sound/core/info_oss.c     |  29 +-
 sound/core/init.c         |  33 +-
 sound/core/seq/Makefile   |   3 +-
 sound/core/seq/seq_info.c |  19 +-
 sound/core/sound.c        |  24 +-
 sound/core/sound_oss.c    |  30 +-
 10 files changed, 373 insertions(+), 585 deletions(-)

-- 
2.3.5

[PATCH 1/8] ALSA: core: Use seq_file for text proc file reads

[Takashi Iwai]

seq_file is _the_ standard interface for simple text proc files.
Though, we still need to support the binary proc files and the text
file write, and also we need to manage the device disconnection
gracefully.  Thus this patch just replaces the text file read code
with seq_file while keeping the rest intact.

snd_iprintf() helper function is now a macro to expand itself to
seq_printf() to be compatible with the existing code.  The seq_file
object is stored to the unused entry->rbuffer->buffer pointer.

When the output size is expected to be large (greater than PAGE_SIZE),
the driver should set entry->size field beforehand.  Then the given
size will be preallocated and the multiple show calls can be avoided.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/info.h |  16 +-
 sound/core/info.c    | 569 ++++++++++++++++++++++-----------------------------
 2 files changed, 261 insertions(+), 324 deletions(-)

diff --git a/include/sound/info.h b/include/sound/info.h
index 9ca1a493d370..ff8962ebece5 100644
--- a/include/sound/info.h
+++ b/include/sound/info.h
@@ -23,6 +23,7 @@
  */
 
 #include <linux/poll.h>
+#include <linux/seq_file.h>
 
 /* buffer for information */
 struct snd_info_buffer {
@@ -110,8 +111,18 @@ void snd_card_info_read_oss(struct snd_info_buffer *buffer);
 static inline void snd_card_info_read_oss(struct snd_info_buffer *buffer) {}
 #endif
 
-__printf(2, 3)
-int snd_iprintf(struct snd_info_buffer *buffer, const char *fmt, ...);
+/**
+ * snd_iprintf - printf on the procfs buffer
+ * @buf: the procfs buffer
+ * @fmt: the printf format
+ *
+ * Outputs the string on the procfs buffer just like printf().
+ *
+ * Return: zero for success, or a negative error code.
+ */
+#define snd_iprintf(buf, fmt, args...) \
+	seq_printf((struct seq_file *)(buf)->buffer, fmt, ##args)
+
 int snd_info_init(void);
 int snd_info_done(void);
 
@@ -175,7 +186,6 @@ static inline int snd_card_proc_new(struct snd_card *card, const char *name,
 static inline void snd_info_set_text_ops(struct snd_info_entry *entry __attribute__((unused)),
 					 void *private_data,
 					 void (*read)(struct snd_info_entry *, struct snd_info_buffer *)) {}
-
 static inline int snd_info_check_reserved_words(const char *str) { return 1; }
 
 #endif
diff --git a/sound/core/info.c b/sound/core/info.c
index 9f404e965ea2..8c1275f0fcbd 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -81,66 +81,6 @@ static int snd_info_version_init(void);
 static int snd_info_version_done(void);
 static void snd_info_disconnect(struct snd_info_entry *entry);
 
-
-/* resize the proc r/w buffer */
-static int resize_info_buffer(struct snd_info_buffer *buffer,
-			      unsigned int nsize)
-{
-	char *nbuf;
-
-	nsize = PAGE_ALIGN(nsize);
-	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL | __GFP_ZERO);
-	if (! nbuf)
-		return -ENOMEM;
-
-	buffer->buffer = nbuf;
-	buffer->len = nsize;
-	return 0;
-}
-
-/**
- * snd_iprintf - printf on the procfs buffer
- * @buffer: the procfs buffer
- * @fmt: the printf format
- *
- * Outputs the string on the procfs buffer just like printf().
- *
- * Return: The size of output string, or a negative error code.
- */
-int snd_iprintf(struct snd_info_buffer *buffer, const char *fmt, ...)
-{
-	va_list args;
-	int len, res;
-	int err = 0;
-
-	might_sleep();
-	if (buffer->stop || buffer->error)
-		return 0;
-	len = buffer->len - buffer->size;
-	va_start(args, fmt);
-	for (;;) {
-		va_list ap;
-		va_copy(ap, args);
-		res = vsnprintf(buffer->buffer + buffer->curr, len, fmt, ap);
-		va_end(ap);
-		if (res < len)
-			break;
-		err = resize_info_buffer(buffer, buffer->len + PAGE_SIZE);
-		if (err < 0)
-			break;
-		len = buffer->len - buffer->size;
-	}
-	va_end(args);
-
-	if (err < 0)
-		return err;
-	buffer->curr += res;
-	buffer->size += res;
-	return res;
-}
-
-EXPORT_SYMBOL(snd_iprintf);
-
 /*
 
  */
@@ -153,6 +93,37 @@ EXPORT_SYMBOL(snd_seq_root);
 struct snd_info_entry *snd_oss_root;
 #endif
 
+static int alloc_info_private(struct snd_info_entry *entry,
+			      struct snd_info_private_data **ret)
+{
+	struct snd_info_private_data *data;
+
+	if (!entry || !entry->p)
+		return -ENODEV;
+	if (!try_module_get(entry->module))
+		return -EFAULT;
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		module_put(entry->module);
+		return -ENOMEM;
+	}
+	data->entry = entry;
+	*ret = data;
+	return 0;
+}
+
+static bool valid_pos(loff_t pos, size_t count)
+{
+	if (pos < 0 || (long) pos != pos || (ssize_t) count < 0)
+		return false;
+	if ((unsigned long) pos + (unsigned long) count < (unsigned long) pos)
+		return false;
+	return true;
+}
+
+/*
+ * file ops for binary proc files
+ */
 static loff_t snd_info_entry_llseek(struct file *file, loff_t offset, int orig)
 {
 	struct snd_info_private_data *data;
@@ -162,17 +133,14 @@ static loff_t snd_info_entry_llseek(struct file *file, loff_t offset, int orig)
 	data = file->private_data;
 	entry = data->entry;
 	mutex_lock(&entry->access);
-	if (entry->content == SNDRV_INFO_CONTENT_DATA &&
-	    entry->c.ops->llseek) {
+	if (entry->c.ops->llseek) {
 		offset = entry->c.ops->llseek(entry,
 					      data->file_private_data,
 					      file, offset, orig);
 		goto out;
 	}
-	if (entry->content == SNDRV_INFO_CONTENT_DATA)
-		size = entry->size;
-	else
-		size = 0;
+
+	size = entry->size;
 	switch (orig) {
 	case SEEK_SET:
 		break;
@@ -201,45 +169,20 @@ static loff_t snd_info_entry_llseek(struct file *file, loff_t offset, int orig)
 static ssize_t snd_info_entry_read(struct file *file, char __user *buffer,
 				   size_t count, loff_t * offset)
 {
-	struct snd_info_private_data *data;
-	struct snd_info_entry *entry;
-	struct snd_info_buffer *buf;
-	size_t size = 0;
+	struct snd_info_private_data *data = file->private_data;
+	struct snd_info_entry *entry = data->entry;
+	size_t size;
 	loff_t pos;
 
-	data = file->private_data;
-	if (snd_BUG_ON(!data))
-		return -ENXIO;
 	pos = *offset;
-	if (pos < 0 || (long) pos != pos || (ssize_t) count < 0)
-		return -EIO;
-	if ((unsigned long) pos + (unsigned long) count < (unsigned long) pos)
+	if (!valid_pos(pos, count))
 		return -EIO;
-	entry = data->entry;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_TEXT:
-		buf = data->rbuffer;
-		if (buf == NULL)
-			return -EIO;
-		if (pos >= buf->size)
-			return 0;
-		size = buf->size - pos;
-		size = min(count, size);
-		if (copy_to_user(buffer, buf->buffer + pos, size))
-			return -EFAULT;
-		break;
-	case SNDRV_INFO_CONTENT_DATA:
-		if (pos >= entry->size)
-			return 0;
-		if (entry->c.ops->read) {
-			size = entry->size - pos;
-			size = min(count, size);
-			size = entry->c.ops->read(entry,
-						  data->file_private_data,
-						  file, buffer, size, pos);
-		}
-		break;
-	}
+	if (pos >= entry->size)
+		return 0;
+	size = entry->size - pos;
+	size = min(count, size);
+	size = entry->c.ops->read(entry, data->file_private_data,
+				  file, buffer, size, pos);
 	if ((ssize_t) size > 0)
 		*offset = pos + size;
 	return size;
@@ -248,280 +191,259 @@ static ssize_t snd_info_entry_read(struct file *file, char __user *buffer,
 static ssize_t snd_info_entry_write(struct file *file, const char __user *buffer,
 				    size_t count, loff_t * offset)
 {
-	struct snd_info_private_data *data;
-	struct snd_info_entry *entry;
-	struct snd_info_buffer *buf;
+	struct snd_info_private_data *data = file->private_data;
+	struct snd_info_entry *entry = data->entry;
 	ssize_t size = 0;
 	loff_t pos;
 
-	data = file->private_data;
-	if (snd_BUG_ON(!data))
-		return -ENXIO;
-	entry = data->entry;
 	pos = *offset;
-	if (pos < 0 || (long) pos != pos || (ssize_t) count < 0)
+	if (!valid_pos(pos, count))
 		return -EIO;
-	if ((unsigned long) pos + (unsigned long) count < (unsigned long) pos)
-		return -EIO;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_TEXT:
-		buf = data->wbuffer;
-		if (buf == NULL)
-			return -EIO;
-		mutex_lock(&entry->access);
-		if (pos + count >= buf->len) {
-			if (resize_info_buffer(buf, pos + count)) {
-				mutex_unlock(&entry->access);
-				return -ENOMEM;
-			}
-		}
-		if (copy_from_user(buf->buffer + pos, buffer, count)) {
-			mutex_unlock(&entry->access);
-			return -EFAULT;
-		}
-		buf->size = pos + count;
-		mutex_unlock(&entry->access);
-		size = count;
-		break;
-	case SNDRV_INFO_CONTENT_DATA:
-		if (entry->c.ops->write && count > 0) {
-			size_t maxsize = entry->size - pos;
-			count = min(count, maxsize);
-			size = entry->c.ops->write(entry,
-						   data->file_private_data,
-						   file, buffer, count, pos);
-		}
-		break;
+	if (count > 0) {
+		size_t maxsize = entry->size - pos;
+		count = min(count, maxsize);
+		size = entry->c.ops->write(entry, data->file_private_data,
+					   file, buffer, count, pos);
 	}
-	if ((ssize_t) size > 0)
+	if (size > 0)
 		*offset = pos + size;
 	return size;
 }
 
-static int snd_info_entry_open(struct inode *inode, struct file *file)
+static unsigned int snd_info_entry_poll(struct file *file, poll_table *wait)
+{
+	struct snd_info_private_data *data = file->private_data;
+	struct snd_info_entry *entry = data->entry;
+	unsigned int mask = 0;
+
+	if (entry->c.ops->poll)
+		return entry->c.ops->poll(entry,
+					  data->file_private_data,
+					  file, wait);
+	if (entry->c.ops->read)
+		mask |= POLLIN | POLLRDNORM;
+	if (entry->c.ops->write)
+		mask |= POLLOUT | POLLWRNORM;
+	return mask;
+}
+
+static long snd_info_entry_ioctl(struct file *file, unsigned int cmd,
+				unsigned long arg)
+{
+	struct snd_info_private_data *data = file->private_data;
+	struct snd_info_entry *entry = data->entry;
+
+	if (!entry->c.ops->ioctl)
+		return -ENOTTY;
+	return entry->c.ops->ioctl(entry, data->file_private_data,
+				   file, cmd, arg);
+}
+
+static int snd_info_entry_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	struct inode *inode = file_inode(file);
+	struct snd_info_private_data *data;
 	struct snd_info_entry *entry;
+
+	data = file->private_data;
+	if (data == NULL)
+		return 0;
+	entry = data->entry;
+	if (!entry->c.ops->mmap)
+		return -ENXIO;
+	return entry->c.ops->mmap(entry, data->file_private_data,
+				  inode, file, vma);
+}
+
+static int snd_info_entry_open(struct inode *inode, struct file *file)
+{
+	struct snd_info_entry *entry = PDE_DATA(inode);
 	struct snd_info_private_data *data;
-	struct snd_info_buffer *buffer;
 	int mode, err;
 
 	mutex_lock(&info_mutex);
-	entry = PDE_DATA(inode);
-	if (entry == NULL || ! entry->p) {
-		mutex_unlock(&info_mutex);
-		return -ENODEV;
-	}
-	if (!try_module_get(entry->module)) {
-		err = -EFAULT;
-		goto __error1;
-	}
+	err = alloc_info_private(entry, &data);
+	if (err < 0)
+		goto unlock;
+
 	mode = file->f_flags & O_ACCMODE;
-	if (mode == O_RDONLY || mode == O_RDWR) {
-		if ((entry->content == SNDRV_INFO_CONTENT_DATA &&
-		     entry->c.ops->read == NULL)) {
-		    	err = -ENODEV;
-		    	goto __error;
-		}
-	}
-	if (mode == O_WRONLY || mode == O_RDWR) {
-		if ((entry->content == SNDRV_INFO_CONTENT_DATA &&
-		     entry->c.ops->write == NULL)) {
-		    	err = -ENODEV;
-		    	goto __error;
-		}
+	if (((mode == O_RDONLY || mode == O_RDWR) && !entry->c.ops->read) ||
+	    ((mode == O_WRONLY || mode == O_RDWR) && !entry->c.ops->write)) {
+		err = -ENODEV;
+		goto error;
 	}
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (data == NULL) {
-		err = -ENOMEM;
-		goto __error;
-	}
-	data->entry = entry;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_TEXT:
-		if (mode == O_RDONLY || mode == O_RDWR) {
-			buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
-			if (buffer == NULL)
-				goto __nomem;
-			data->rbuffer = buffer;
-			buffer->len = PAGE_SIZE;
-			buffer->buffer = kzalloc(buffer->len, GFP_KERNEL);
-			if (buffer->buffer == NULL)
-				goto __nomem;
-		}
-		if (mode == O_WRONLY || mode == O_RDWR) {
-			buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
-			if (buffer == NULL)
-				goto __nomem;
-			data->wbuffer = buffer;
-			buffer->len = PAGE_SIZE;
-			buffer->buffer = kmalloc(buffer->len, GFP_KERNEL);
-			if (buffer->buffer == NULL)
-				goto __nomem;
-		}
-		break;
-	case SNDRV_INFO_CONTENT_DATA:	/* data */
-		if (entry->c.ops->open) {
-			if ((err = entry->c.ops->open(entry, mode,
-						      &data->file_private_data)) < 0) {
-				kfree(data);
-				goto __error;
-			}
-		}
-		break;
+
+	if (entry->c.ops->open) {
+		err = entry->c.ops->open(entry, mode, &data->file_private_data);
+		if (err < 0)
+			goto error;
 	}
+
 	file->private_data = data;
 	mutex_unlock(&info_mutex);
-	if (entry->content == SNDRV_INFO_CONTENT_TEXT &&
-	    (mode == O_RDONLY || mode == O_RDWR)) {
-		if (entry->c.text.read) {
-			mutex_lock(&entry->access);
-			entry->c.text.read(entry, data->rbuffer);
-			mutex_unlock(&entry->access);
-		}
-	}
 	return 0;
 
- __nomem:
-	if (data->rbuffer) {
-		kfree(data->rbuffer->buffer);
-		kfree(data->rbuffer);
-	}
-	if (data->wbuffer) {
-		kfree(data->wbuffer->buffer);
-		kfree(data->wbuffer);
-	}
+ error:
 	kfree(data);
-	err = -ENOMEM;
-      __error:
 	module_put(entry->module);
-      __error1:
+ unlock:
 	mutex_unlock(&info_mutex);
 	return err;
 }
 
 static int snd_info_entry_release(struct inode *inode, struct file *file)
 {
-	struct snd_info_entry *entry;
-	struct snd_info_private_data *data;
-	int mode;
+	struct snd_info_private_data *data = file->private_data;
+	struct snd_info_entry *entry = data->entry;
 
-	mode = file->f_flags & O_ACCMODE;
-	data = file->private_data;
-	entry = data->entry;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_TEXT:
-		if (data->rbuffer) {
-			kfree(data->rbuffer->buffer);
-			kfree(data->rbuffer);
-		}
-		if (data->wbuffer) {
-			if (entry->c.text.write) {
-				entry->c.text.write(entry, data->wbuffer);
-				if (data->wbuffer->error) {
-					if (entry->card)
-						dev_warn(entry->card->dev, "info: data write error to %s (%i)\n",
-							 entry->name,
-							 data->wbuffer->error);
-					else
-						pr_warn("ALSA: info: data write error to %s (%i)\n",
-							entry->name,
-							data->wbuffer->error);
-				}
-			}
-			kfree(data->wbuffer->buffer);
-			kfree(data->wbuffer);
-		}
-		break;
-	case SNDRV_INFO_CONTENT_DATA:
-		if (entry->c.ops->release)
-			entry->c.ops->release(entry, mode,
-					      data->file_private_data);
-		break;
-	}
+	if (entry->c.ops->release)
+		entry->c.ops->release(entry, file->f_flags & O_ACCMODE,
+				      data->file_private_data);
 	module_put(entry->module);
 	kfree(data);
 	return 0;
 }
 
-static unsigned int snd_info_entry_poll(struct file *file, poll_table * wait)
+static const struct file_operations snd_info_entry_operations =
 {
-	struct snd_info_private_data *data;
-	struct snd_info_entry *entry;
-	unsigned int mask;
+	.owner =		THIS_MODULE,
+	.llseek =		snd_info_entry_llseek,
+	.read =			snd_info_entry_read,
+	.write =		snd_info_entry_write,
+	.poll =			snd_info_entry_poll,
+	.unlocked_ioctl =	snd_info_entry_ioctl,
+	.mmap =			snd_info_entry_mmap,
+	.open =			snd_info_entry_open,
+	.release =		snd_info_entry_release,
+};
 
-	data = file->private_data;
-	if (data == NULL)
-		return 0;
-	entry = data->entry;
-	mask = 0;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_DATA:
-		if (entry->c.ops->poll)
-			return entry->c.ops->poll(entry,
-						  data->file_private_data,
-						  file, wait);
-		if (entry->c.ops->read)
-			mask |= POLLIN | POLLRDNORM;
-		if (entry->c.ops->write)
-			mask |= POLLOUT | POLLWRNORM;
-		break;
+/*
+ * file ops for text proc files
+ */
+static ssize_t snd_info_text_entry_write(struct file *file,
+					 const char __user *buffer,
+					 size_t count, loff_t *offset)
+{
+	struct seq_file *m = file->private_data;
+	struct snd_info_private_data *data = m->private;
+	struct snd_info_entry *entry = data->entry;
+	struct snd_info_buffer *buf;
+	loff_t pos;
+	size_t next;
+	int err = 0;
+
+	pos = *offset;
+	if (!valid_pos(pos, count))
+		return -EIO;
+	next = pos + count;
+	mutex_lock(&entry->access);
+	buf = data->wbuffer;
+	if (!buf) {
+		data->wbuffer = buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+		if (!buf) {
+			err = -ENOMEM;
+			goto error;
+		}
 	}
-	return mask;
+	if (next > buf->len) {
+		char *nbuf = krealloc(buf->buffer, PAGE_ALIGN(next),
+				      GFP_KERNEL | __GFP_ZERO);
+		if (!nbuf) {
+			err = -ENOMEM;
+			goto error;
+		}
+		buf->buffer = nbuf;
+		buf->len = PAGE_ALIGN(next);
+	}
+	if (copy_from_user(buf->buffer + pos, buffer, count)) {
+		err = -EFAULT;
+		goto error;
+	}
+	buf->size = next;
+ error:
+	mutex_unlock(&entry->access);
+	if (err < 0)
+		return err;
+	*offset = next;
+	return count;
 }
 
-static long snd_info_entry_ioctl(struct file *file, unsigned int cmd,
-				unsigned long arg)
+static int snd_info_seq_show(struct seq_file *seq, void *p)
 {
-	struct snd_info_private_data *data;
-	struct snd_info_entry *entry;
+	struct snd_info_private_data *data = seq->private;
+	struct snd_info_entry *entry = data->entry;
 
-	data = file->private_data;
-	if (data == NULL)
-		return 0;
-	entry = data->entry;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_DATA:
-		if (entry->c.ops->ioctl)
-			return entry->c.ops->ioctl(entry,
-						   data->file_private_data,
-						   file, cmd, arg);
-		break;
+	if (entry->c.text.read) {
+		data->rbuffer->buffer = (char *)seq; /* XXX hack! */
+		entry->c.text.read(entry, data->rbuffer);
 	}
-	return -ENOTTY;
+	return 0;
 }
 
-static int snd_info_entry_mmap(struct file *file, struct vm_area_struct *vma)
+static int snd_info_text_entry_open(struct inode *inode, struct file *file)
 {
-	struct inode *inode = file_inode(file);
+	struct snd_info_entry *entry = PDE_DATA(inode);
 	struct snd_info_private_data *data;
-	struct snd_info_entry *entry;
+	int err;
 
-	data = file->private_data;
-	if (data == NULL)
-		return 0;
-	entry = data->entry;
-	switch (entry->content) {
-	case SNDRV_INFO_CONTENT_DATA:
-		if (entry->c.ops->mmap)
-			return entry->c.ops->mmap(entry,
-						  data->file_private_data,
-						  inode, file, vma);
-		break;
+	mutex_lock(&info_mutex);
+	err = alloc_info_private(entry, &data);
+	if (err < 0)
+		goto unlock;
+
+	data->rbuffer = kzalloc(sizeof(*data->rbuffer), GFP_KERNEL);
+	if (!data->rbuffer) {
+		err = -ENOMEM;
+		goto error;
+	}
+	if (entry->size)
+		err = single_open_size(file, snd_info_seq_show, data,
+				       entry->size);
+	else
+		err = single_open(file, snd_info_seq_show, data);
+	if (err < 0)
+		goto error;
+	mutex_unlock(&info_mutex);
+	return 0;
+
+ error:
+	kfree(data->rbuffer);
+	kfree(data);
+	module_put(entry->module);
+ unlock:
+	mutex_unlock(&info_mutex);
+	return err;
+}
+
+static int snd_info_text_entry_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *m = file->private_data;
+	struct snd_info_private_data *data = m->private;
+	struct snd_info_entry *entry = data->entry;
+
+	if (data->wbuffer && entry->c.text.write)
+		entry->c.text.write(entry, data->wbuffer);
+
+	single_release(inode, file);
+	kfree(data->rbuffer);
+	if (data->wbuffer) {
+		kfree(data->wbuffer->buffer);
+		kfree(data->wbuffer);
 	}
-	return -ENXIO;
+
+	module_put(entry->module);
+	kfree(data);
+	return 0;
 }
 
-static const struct file_operations snd_info_entry_operations =
+static const struct file_operations snd_info_text_entry_ops =
 {
 	.owner =		THIS_MODULE,
-	.llseek =		snd_info_entry_llseek,
-	.read =			snd_info_entry_read,
-	.write =		snd_info_entry_write,
-	.poll =			snd_info_entry_poll,
-	.unlocked_ioctl =	snd_info_entry_ioctl,
-	.mmap =			snd_info_entry_mmap,
-	.open =			snd_info_entry_open,
-	.release =		snd_info_entry_release,
+	.open =			snd_info_text_entry_open,
+	.release =		snd_info_text_entry_release,
+	.write =		snd_info_text_entry_write,
+	.llseek =		seq_lseek,
+	.read =			seq_read,
 };
 
 int __init snd_info_init(void)
@@ -955,8 +877,13 @@ int snd_info_register(struct snd_info_entry * entry)
 			return -ENOMEM;
 		}
 	} else {
+		const struct file_operations *ops;
+		if (entry->content == SNDRV_INFO_CONTENT_DATA)
+			ops = &snd_info_entry_operations;
+		else
+			ops = &snd_info_text_entry_ops;
 		p = proc_create_data(entry->name, entry->mode, root,
-					&snd_info_entry_operations, entry);
+				     ops, entry);
 		if (!p) {
 			mutex_unlock(&info_mutex);
 			return -ENOMEM;
-- 
2.3.5

[PATCH 2/8] ALSA: core: Fix possible memory leaks at error path in info.c

[Takashi Iwai]

snd_info_init() just returns an error without releasing the previously
assigned resources at error path.  The assigned proc and info entries
have to be released properly.

While we are at it, refactor the code a bit, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/info.c | 62 +++++++++++++++++++++++++++----------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index 8c1275f0fcbd..9c6db5c24da7 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -446,6 +446,22 @@ static const struct file_operations snd_info_text_entry_ops =
 	.read =			seq_read,
 };
 
+static struct snd_info_entry *create_subdir(struct module *mod,
+					    const char *name)
+{
+	struct snd_info_entry *entry;
+
+	entry = snd_info_create_module_entry(mod, name, NULL);
+	if (!entry)
+		return NULL;
+	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	if (snd_info_register(entry) < 0) {
+		snd_info_free_entry(entry);
+		return NULL;
+	}
+	return entry;
+}
+
 int __init snd_info_init(void)
 {
 	struct proc_dir_entry *p;
@@ -455,36 +471,27 @@ int __init snd_info_init(void)
 		return -ENOMEM;
 	snd_proc_root = p;
 #ifdef CONFIG_SND_OSSEMUL
-	{
-		struct snd_info_entry *entry;
-		if ((entry = snd_info_create_module_entry(THIS_MODULE, "oss", NULL)) == NULL)
-			return -ENOMEM;
-		entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
-		if (snd_info_register(entry) < 0) {
-			snd_info_free_entry(entry);
-			return -ENOMEM;
-		}
-		snd_oss_root = entry;
-	}
+	snd_oss_root = create_subdir(THIS_MODULE, "oss");
+	if (!snd_oss_root)
+		goto error;
 #endif
 #if IS_ENABLED(CONFIG_SND_SEQUENCER)
-	{
-		struct snd_info_entry *entry;
-		if ((entry = snd_info_create_module_entry(THIS_MODULE, "seq", NULL)) == NULL)
-			return -ENOMEM;
-		entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
-		if (snd_info_register(entry) < 0) {
-			snd_info_free_entry(entry);
-			return -ENOMEM;
-		}
-		snd_seq_root = entry;
-	}
+	snd_seq_root = create_subdir(THIS_MODULE, "seq");
+	if (!snd_seq_root)
+		goto error;
 #endif
 	snd_info_version_init();
 	snd_minor_info_init();
 	snd_minor_info_oss_init();
 	snd_card_info_init();
 	return 0;
+
+ error:
+#ifdef CONFIG_SND_OSSEMUL
+	snd_info_free_entry(snd_oss_root);
+#endif
+	proc_remove(snd_proc_root);
+	return -ENOMEM;
 }
 
 int __exit snd_info_done(void)
@@ -523,13 +530,9 @@ int snd_info_card_create(struct snd_card *card)
 		return -ENXIO;
 
 	sprintf(str, "card%i", card->number);
-	if ((entry = snd_info_create_module_entry(card->module, str, NULL)) == NULL)
-		return -ENOMEM;
-	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
-	if (snd_info_register(entry) < 0) {
-		snd_info_free_entry(entry);
+	entry = create_subdir(card->module, str);
+	if (!entry)
 		return -ENOMEM;
-	}
 	card->proc_root = entry;
 	return 0;
 }
@@ -758,7 +761,6 @@ EXPORT_SYMBOL(snd_info_create_card_entry);
 static void snd_info_disconnect(struct snd_info_entry *entry)
 {
 	struct list_head *p, *n;
-	struct proc_dir_entry *root;
 
 	list_for_each_safe(p, n, &entry->children) {
 		snd_info_disconnect(list_entry(p, struct snd_info_entry, list));
@@ -767,8 +769,6 @@ static void snd_info_disconnect(struct snd_info_entry *entry)
 	if (! entry->p)
 		return;
 	list_del_init(&entry->list);
-	root = entry->parent == NULL ? snd_proc_root : entry->parent->p;
-	snd_BUG_ON(!root);
 	proc_remove(entry->p);
 	entry->p = NULL;
 }
-- 
2.3.5

[PATCH 3/8] ALSA: core: Remove child proc file elements recursively

[Takashi Iwai]

This patch changes the way to managing the resource release of proc
files: namely, let snd_info_free_entry() freeing the whole children.
Then we can drop the snd_device_*() management and snd_card_proc_new()
can be merely a wrapper to snd_info_create_card_entry().

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/info.h |  9 ++++--
 sound/core/info.c    | 79 +++++++++-------------------------------------------
 2 files changed, 20 insertions(+), 68 deletions(-)

diff --git a/include/sound/info.h b/include/sound/info.h
index ff8962ebece5..3e2fda3c75ee 100644
--- a/include/sound/info.h
+++ b/include/sound/info.h
@@ -24,6 +24,7 @@
 
 #include <linux/poll.h>
 #include <linux/seq_file.h>
+#include <sound/core.h>
 
 /* buffer for information */
 struct snd_info_buffer {
@@ -146,8 +147,12 @@ void snd_info_card_id_change(struct snd_card *card);
 int snd_info_register(struct snd_info_entry *entry);
 
 /* for card drivers */
-int snd_card_proc_new(struct snd_card *card, const char *name,
-		      struct snd_info_entry **entryp);
+static inline int snd_card_proc_new(struct snd_card *card, const char *name,
+				    struct snd_info_entry **entryp)
+{
+	*entryp = snd_info_create_card_entry(card, name, card->proc_root);
+	return *entryp ? 0 : -ENOMEM;
+}
 
 static inline void snd_info_set_text_ops(struct snd_info_entry *entry, 
 	void *private_data,
diff --git a/sound/core/info.c b/sound/core/info.c
index 9c6db5c24da7..96451a130199 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -760,92 +760,39 @@ EXPORT_SYMBOL(snd_info_create_card_entry);
 
 static void snd_info_disconnect(struct snd_info_entry *entry)
 {
-	struct list_head *p, *n;
+	struct snd_info_entry *p, *n;
 
-	list_for_each_safe(p, n, &entry->children) {
-		snd_info_disconnect(list_entry(p, struct snd_info_entry, list));
-	}
-
-	if (! entry->p)
+	if (!entry->p)
 		return;
+	list_for_each_entry_safe(p, n, &entry->children, list)
+		snd_info_disconnect(p);
 	list_del_init(&entry->list);
 	proc_remove(entry->p);
 	entry->p = NULL;
 }
 
-static int snd_info_dev_free_entry(struct snd_device *device)
-{
-	struct snd_info_entry *entry = device->device_data;
-	snd_info_free_entry(entry);
-	return 0;
-}
-
-static int snd_info_dev_register_entry(struct snd_device *device)
-{
-	struct snd_info_entry *entry = device->device_data;
-	return snd_info_register(entry);
-}
-
-/**
- * snd_card_proc_new - create an info entry for the given card
- * @card: the card instance
- * @name: the file name
- * @entryp: the pointer to store the new info entry
- *
- * Creates a new info entry and assigns it to the given card.
- * Unlike snd_info_create_card_entry(), this function registers the
- * info entry as an ALSA device component, so that it can be
- * unregistered/released without explicit call.
- * Also, you don't have to register this entry via snd_info_register(),
- * since this will be registered by snd_card_register() automatically.
- *
- * The parent is assumed as card->proc_root.
- *
- * For releasing this entry, use snd_device_free() instead of
- * snd_info_free_entry(). 
- *
- * Return: Zero if successful, or a negative error code on failure.
- */
-int snd_card_proc_new(struct snd_card *card, const char *name,
-		      struct snd_info_entry **entryp)
-{
-	static struct snd_device_ops ops = {
-		.dev_free = snd_info_dev_free_entry,
-		.dev_register =	snd_info_dev_register_entry,
-		/* disconnect is done via snd_info_card_disconnect() */
-	};
-	struct snd_info_entry *entry;
-	int err;
-
-	entry = snd_info_create_card_entry(card, name, card->proc_root);
-	if (! entry)
-		return -ENOMEM;
-	if ((err = snd_device_new(card, SNDRV_DEV_INFO, entry, &ops)) < 0) {
-		snd_info_free_entry(entry);
-		return err;
-	}
-	if (entryp)
-		*entryp = entry;
-	return 0;
-}
-
-EXPORT_SYMBOL(snd_card_proc_new);
-
 /**
  * snd_info_free_entry - release the info entry
  * @entry: the info entry
  *
- * Releases the info entry.  Don't call this after registered.
+ * Releases the info entry.
  */
 void snd_info_free_entry(struct snd_info_entry * entry)
 {
-	if (entry == NULL)
+	struct snd_info_entry *p, *n;
+
+	if (!entry)
 		return;
 	if (entry->p) {
 		mutex_lock(&info_mutex);
 		snd_info_disconnect(entry);
 		mutex_unlock(&info_mutex);
 	}
+
+	/* free all children at first */
+	list_for_each_entry_safe(p, n, &entry->children, list)
+		snd_info_free_entry(p);
+
 	kfree(entry->name);
 	if (entry->private_free)
 		entry->private_free(entry);
-- 
2.3.5

[PATCH 4/8] ALSA: core: Manage asound root directory with snd_info_entry

[Takashi Iwai]

This makes easier to release the all children.  Further cleanups will
follow.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/info.c | 52 ++++++++++++++--------------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index 96451a130199..55c626eeb061 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -78,14 +78,13 @@ struct snd_info_private_data {
 };
 
 static int snd_info_version_init(void);
-static int snd_info_version_done(void);
 static void snd_info_disconnect(struct snd_info_entry *entry);
 
 /*
 
  */
 
-static struct proc_dir_entry *snd_proc_root;
+static struct snd_info_entry *snd_proc_root;
 struct snd_info_entry *snd_seq_root;
 EXPORT_SYMBOL(snd_seq_root);
 
@@ -462,14 +461,17 @@ static struct snd_info_entry *create_subdir(struct module *mod,
 	return entry;
 }
 
+static struct snd_info_entry *snd_info_create_entry(const char *name);
+
 int __init snd_info_init(void)
 {
-	struct proc_dir_entry *p;
-
-	p = proc_mkdir("asound", NULL);
-	if (p == NULL)
+	snd_proc_root = snd_info_create_entry("asound");
+	if (!snd_proc_root)
 		return -ENOMEM;
-	snd_proc_root = p;
+	snd_proc_root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	snd_proc_root->p = proc_mkdir("asound", NULL);
+	if (!snd_proc_root->p)
+		goto error;
 #ifdef CONFIG_SND_OSSEMUL
 	snd_oss_root = create_subdir(THIS_MODULE, "oss");
 	if (!snd_oss_root)
@@ -487,10 +489,7 @@ int __init snd_info_init(void)
 	return 0;
 
  error:
-#ifdef CONFIG_SND_OSSEMUL
-	snd_info_free_entry(snd_oss_root);
-#endif
-	proc_remove(snd_proc_root);
+	snd_info_free_entry(snd_proc_root);
 	return -ENOMEM;
 }
 
@@ -499,25 +498,11 @@ int __exit snd_info_done(void)
 	snd_card_info_done();
 	snd_minor_info_oss_done();
 	snd_minor_info_done();
-	snd_info_version_done();
-	if (snd_proc_root) {
-#if IS_ENABLED(CONFIG_SND_SEQUENCER)
-		snd_info_free_entry(snd_seq_root);
-#endif
-#ifdef CONFIG_SND_OSSEMUL
-		snd_info_free_entry(snd_oss_root);
-#endif
-		proc_remove(snd_proc_root);
-	}
+	snd_info_free_entry(snd_proc_root);
 	return 0;
 }
 
 /*
-
- */
-
-
-/*
  * create a card proc file
  * called from init.c
  */
@@ -551,7 +536,7 @@ int snd_info_card_register(struct snd_card *card)
 	if (!strcmp(card->id, card->proc_root->name))
 		return 0;
 
-	p = proc_symlink(card->id, snd_proc_root, card->proc_root->name);
+	p = proc_symlink(card->id, snd_proc_root->p, card->proc_root->name);
 	if (p == NULL)
 		return -ENOMEM;
 	card->proc_root_link = p;
@@ -570,7 +555,7 @@ void snd_info_card_id_change(struct snd_card *card)
 	}
 	if (strcmp(card->id, card->proc_root->name))
 		card->proc_root_link = proc_symlink(card->id,
-						    snd_proc_root,
+						    snd_proc_root->p,
 						    card->proc_root->name);
 	mutex_unlock(&info_mutex);
 }
@@ -815,7 +800,7 @@ int snd_info_register(struct snd_info_entry * entry)
 
 	if (snd_BUG_ON(!entry))
 		return -ENXIO;
-	root = entry->parent == NULL ? snd_proc_root : entry->parent->p;
+	root = entry->parent == NULL ? snd_proc_root->p : entry->parent->p;
 	mutex_lock(&info_mutex);
 	if (S_ISDIR(entry->mode)) {
 		p = proc_mkdir_mode(entry->name, entry->mode, root);
@@ -850,8 +835,6 @@ EXPORT_SYMBOL(snd_info_register);
 
  */
 
-static struct snd_info_entry *snd_info_version_entry;
-
 static void snd_info_version_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
 {
 	snd_iprintf(buffer,
@@ -871,13 +854,6 @@ static int __init snd_info_version_init(void)
 		snd_info_free_entry(entry);
 		return -ENOMEM;
 	}
-	snd_info_version_entry = entry;
-	return 0;
-}
-
-static int __exit snd_info_version_done(void)
-{
-	snd_info_free_entry(snd_info_version_entry);
 	return 0;
 }
 
-- 
2.3.5

[PATCH 5/8] ALSA: core: Remove superfluous exit calls for proc entries

[Takashi Iwai]

Since each proc entry is freed automatically by the parent, we don't
have to take care of its life cycle any longer.  This allows us to
reduce a few more lines of codes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/core.h   |  4 ----
 sound/core/info.c      |  3 ---
 sound/core/init.c      | 17 -----------------
 sound/core/sound.c     | 10 ----------
 sound/core/sound_oss.c | 14 +-------------
 5 files changed, 1 insertion(+), 47 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index b12931f513f4..cdfecafff0f4 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -224,16 +224,13 @@ void *snd_lookup_oss_minor_data(unsigned int minor, int type);
 #endif
 
 int snd_minor_info_init(void);
-int snd_minor_info_done(void);
 
 /* sound_oss.c */
 
 #ifdef CONFIG_SND_OSSEMUL
 int snd_minor_info_oss_init(void);
-int snd_minor_info_oss_done(void);
 #else
 static inline int snd_minor_info_oss_init(void) { return 0; }
-static inline int snd_minor_info_oss_done(void) { return 0; }
 #endif
 
 /* memory.c */
@@ -262,7 +259,6 @@ int snd_card_free_when_closed(struct snd_card *card);
 void snd_card_set_id(struct snd_card *card, const char *id);
 int snd_card_register(struct snd_card *card);
 int snd_card_info_init(void);
-int snd_card_info_done(void);
 int snd_card_add_dev_attr(struct snd_card *card,
 			  const struct attribute_group *group);
 int snd_component_add(struct snd_card *card, const char *component);
diff --git a/sound/core/info.c b/sound/core/info.c
index 55c626eeb061..339f90a3aa29 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -495,9 +495,6 @@ int __init snd_info_init(void)
 
 int __exit snd_info_done(void)
 {
-	snd_card_info_done();
-	snd_minor_info_oss_done();
-	snd_minor_info_done();
 	snd_info_free_entry(snd_proc_root);
 	return 0;
 }
diff --git a/sound/core/init.c b/sound/core/init.c
index 04734e047bfe..0af34fac0499 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -783,8 +783,6 @@ int snd_card_register(struct snd_card *card)
 EXPORT_SYMBOL(snd_card_register);
 
 #ifdef CONFIG_PROC_FS
-static struct snd_info_entry *snd_card_info_entry;
-
 static void snd_card_info_read(struct snd_info_entry *entry,
 			       struct snd_info_buffer *buffer)
 {
@@ -810,7 +808,6 @@ static void snd_card_info_read(struct snd_info_entry *entry,
 }
 
 #ifdef CONFIG_SND_OSSEMUL
-
 void snd_card_info_read_oss(struct snd_info_buffer *buffer)
 {
 	int idx, count;
@@ -832,7 +829,6 @@ void snd_card_info_read_oss(struct snd_info_buffer *buffer)
 #endif
 
 #ifdef MODULE
-static struct snd_info_entry *snd_card_module_info_entry;
 static void snd_card_module_info_read(struct snd_info_entry *entry,
 				      struct snd_info_buffer *buffer)
 {
@@ -861,7 +857,6 @@ int __init snd_card_info_init(void)
 		snd_info_free_entry(entry);
 		return -ENOMEM;
 	}
-	snd_card_info_entry = entry;
 
 #ifdef MODULE
 	entry = snd_info_create_module_entry(THIS_MODULE, "modules", NULL);
@@ -869,23 +864,11 @@ int __init snd_card_info_init(void)
 		entry->c.text.read = snd_card_module_info_read;
 		if (snd_info_register(entry) < 0)
 			snd_info_free_entry(entry);
-		else
-			snd_card_module_info_entry = entry;
 	}
 #endif
 
 	return 0;
 }
-
-int __exit snd_card_info_done(void)
-{
-	snd_info_free_entry(snd_card_info_entry);
-#ifdef MODULE
-	snd_info_free_entry(snd_card_module_info_entry);
-#endif
-	return 0;
-}
-
 #endif /* CONFIG_PROC_FS */
 
 /**
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 5fc93d00572a..d584944c8fe5 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -334,9 +334,6 @@ EXPORT_SYMBOL(snd_unregister_device);
 /*
  *  INFO PART
  */
-
-static struct snd_info_entry *snd_minor_info_entry;
-
 static const char *snd_device_type_name(int type)
 {
 	switch (type) {
@@ -396,13 +393,6 @@ int __init snd_minor_info_init(void)
 			entry = NULL;
 		}
 	}
-	snd_minor_info_entry = entry;
-	return 0;
-}
-
-int __exit snd_minor_info_done(void)
-{
-	snd_info_free_entry(snd_minor_info_entry);
 	return 0;
 }
 #endif /* CONFIG_PROC_FS */
diff --git a/sound/core/sound_oss.c b/sound/core/sound_oss.c
index 573a65eb2b79..5fc3c6534225 100644
--- a/sound/core/sound_oss.c
+++ b/sound/core/sound_oss.c
@@ -214,9 +214,6 @@ EXPORT_SYMBOL(snd_unregister_oss_device);
  */
 
 #ifdef CONFIG_PROC_FS
-
-static struct snd_info_entry *snd_minor_info_oss_entry;
-
 static const char *snd_oss_device_type_name(int type)
 {
 	switch (type) {
@@ -265,18 +262,9 @@ int __init snd_minor_info_oss_init(void)
 	entry = snd_info_create_module_entry(THIS_MODULE, "devices", snd_oss_root);
 	if (entry) {
 		entry->c.text.read = snd_minor_info_oss_read;
-		if (snd_info_register(entry) < 0) {
+		if (snd_info_register(entry) < 0)
 			snd_info_free_entry(entry);
-			entry = NULL;
-		}
 	}
-	snd_minor_info_oss_entry = entry;
-	return 0;
-}
-
-int __exit snd_minor_info_oss_done(void)
-{
-	snd_info_free_entry(snd_minor_info_oss_entry);
 	return 0;
 }
 #endif /* CONFIG_PROC_FS */
-- 
2.3.5

[PATCH 6/8] ALSA: core: Don't ignore errors at creating proc files

[Takashi Iwai]

We ignore the errors at creating proc files in many places.  But they
should be rather treated seriously.

Also, by assuring the error handling, we can get rid of superfluous
snd_info_free_entry() calls as they will be removed by the parent in
the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/info.c         | 15 ++++++---------
 sound/core/init.c         | 16 +++++++---------
 sound/core/seq/seq_info.c | 17 ++++++++++++++---
 sound/core/sound.c        | 12 ++++--------
 sound/core/sound_oss.c    | 10 ++++------
 5 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index 339f90a3aa29..4169062fabf5 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -482,10 +482,11 @@ int __init snd_info_init(void)
 	if (!snd_seq_root)
 		goto error;
 #endif
-	snd_info_version_init();
-	snd_minor_info_init();
-	snd_minor_info_oss_init();
-	snd_card_info_init();
+	if (snd_info_version_init() < 0 ||
+	    snd_minor_info_init() < 0 ||
+	    snd_minor_info_oss_init() < 0 ||
+	    snd_card_info_init() < 0)
+		goto error;
 	return 0;
 
  error:
@@ -847,11 +848,7 @@ static int __init snd_info_version_init(void)
 	if (entry == NULL)
 		return -ENOMEM;
 	entry->c.text.read = snd_info_version_read;
-	if (snd_info_register(entry) < 0) {
-		snd_info_free_entry(entry);
-		return -ENOMEM;
-	}
-	return 0;
+	return snd_info_register(entry); /* freed in error path */
 }
 
 #endif /* CONFIG_PROC_FS */
diff --git a/sound/core/init.c b/sound/core/init.c
index 0af34fac0499..769a783757ff 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -853,18 +853,16 @@ int __init snd_card_info_init(void)
 	if (! entry)
 		return -ENOMEM;
 	entry->c.text.read = snd_card_info_read;
-	if (snd_info_register(entry) < 0) {
-		snd_info_free_entry(entry);
-		return -ENOMEM;
-	}
+	if (snd_info_register(entry) < 0)
+		return -ENOMEM; /* freed in error path */
 
 #ifdef MODULE
 	entry = snd_info_create_module_entry(THIS_MODULE, "modules", NULL);
-	if (entry) {
-		entry->c.text.read = snd_card_module_info_read;
-		if (snd_info_register(entry) < 0)
-			snd_info_free_entry(entry);
-	}
+	if (!entry)
+		return -ENOMEM;
+	entry->c.text.read = snd_card_module_info_read;
+	if (snd_info_register(entry) < 0)
+		return -ENOMEM; /* freed in error path */
 #endif
 
 	return 0;
diff --git a/sound/core/seq/seq_info.c b/sound/core/seq/seq_info.c
index acf7769419f0..d3c65e780e9e 100644
--- a/sound/core/seq/seq_info.c
+++ b/sound/core/seq/seq_info.c
@@ -51,6 +51,13 @@ create_info_entry(char *name, void (*read)(struct snd_info_entry *,
 	return entry;
 }
 
+static void free_info_entries(void)
+{
+	snd_info_free_entry(queues_entry);
+	snd_info_free_entry(clients_entry);
+	snd_info_free_entry(timer_entry);
+}
+
 /* create all our /proc entries */
 int __init snd_seq_info_init(void)
 {
@@ -59,14 +66,18 @@ int __init snd_seq_info_init(void)
 	clients_entry = create_info_entry("clients",
 					  snd_seq_info_clients_read);
 	timer_entry = create_info_entry("timer", snd_seq_info_timer_read);
+	if (!queues_entry || !clients_entry || !timer_entry)
+		goto error;
 	return 0;
+
+ error:
+	free_info_entries();
+	return -ENOMEM;
 }
 
 int __exit snd_seq_info_done(void)
 {
-	snd_info_free_entry(queues_entry);
-	snd_info_free_entry(clients_entry);
-	snd_info_free_entry(timer_entry);
+	free_info_entries();
 	return 0;
 }
 #endif
diff --git a/sound/core/sound.c b/sound/core/sound.c
index d584944c8fe5..8fc402e4ff35 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -386,14 +386,10 @@ int __init snd_minor_info_init(void)
 	struct snd_info_entry *entry;
 
 	entry = snd_info_create_module_entry(THIS_MODULE, "devices", NULL);
-	if (entry) {
-		entry->c.text.read = snd_minor_info_read;
-		if (snd_info_register(entry) < 0) {
-			snd_info_free_entry(entry);
-			entry = NULL;
-		}
-	}
-	return 0;
+	if (!entry)
+		return -ENOMEM;
+	entry->c.text.read = snd_minor_info_read;
+	return snd_info_register(entry); /* freed in error path */
 }
 #endif /* CONFIG_PROC_FS */
 
diff --git a/sound/core/sound_oss.c b/sound/core/sound_oss.c
index 5fc3c6534225..56d2f409f1ef 100644
--- a/sound/core/sound_oss.c
+++ b/sound/core/sound_oss.c
@@ -260,12 +260,10 @@ int __init snd_minor_info_oss_init(void)
 	struct snd_info_entry *entry;
 
 	entry = snd_info_create_module_entry(THIS_MODULE, "devices", snd_oss_root);
-	if (entry) {
-		entry->c.text.read = snd_minor_info_oss_read;
-		if (snd_info_register(entry) < 0)
-			snd_info_free_entry(entry);
-	}
-	return 0;
+	if (!entry)
+		return -ENOMEM;
+	entry->c.text.read = snd_minor_info_oss_read;
+	return snd_info_register(entry); /* freed in error path */
 }
 #endif /* CONFIG_PROC_FS */
 
-- 
2.3.5

[PATCH 7/8] ALSA: core: Build conditionally and remove superfluous ifdefs

[Takashi Iwai]

Minor cleanups of Makefile to build some codes conditionally so that
a few ifdefs can be reduced.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/Makefile       | 8 ++++++--
 sound/core/info.c         | 8 --------
 sound/core/info_oss.c     | 4 ----
 sound/core/seq/Makefile   | 3 ++-
 sound/core/seq/seq_info.c | 2 --
 sound/core/sound_oss.c    | 8 --------
 6 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/sound/core/Makefile b/sound/core/Makefile
index 4daf2f58261c..ae1d32b084fd 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -3,9 +3,13 @@
 # Copyright (c) 1999,2001 by Jaroslav Kysela <perex@perex.cz>
 #
 
-snd-y     := sound.o init.o memory.o info.o control.o misc.o device.o
+snd-y     := sound.o init.o memory.o control.o misc.o device.o
+ifneq ($(CONFIG_PROC_FS),)
+snd-y += info.o
+snd-$(CONFIG_SND_OSSEMUL) += info_oss.o
+endif
 snd-$(CONFIG_ISA_DMA_API) += isadma.o
-snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o
+snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o
 snd-$(CONFIG_SND_VMASTER) += vmaster.o
 snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o
 snd-$(CONFIG_SND_JACK)	  += jack.o
diff --git a/sound/core/info.c b/sound/core/info.c
index 4169062fabf5..f8bdd9b6f322 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -33,12 +33,6 @@
 #include <linux/mutex.h>
 #include <stdarg.h>
 
-/*
- *
- */
-
-#ifdef CONFIG_PROC_FS
-
 int snd_info_check_reserved_words(const char *str)
 {
 	static char *reserved[] =
@@ -850,5 +844,3 @@ static int __init snd_info_version_init(void)
 	entry->c.text.read = snd_info_version_read;
 	return snd_info_register(entry); /* freed in error path */
 }
-
-#endif /* CONFIG_PROC_FS */
diff --git a/sound/core/info_oss.c b/sound/core/info_oss.c
index 83c29dbff9c0..bd4d2c6233c2 100644
--- a/sound/core/info_oss.c
+++ b/sound/core/info_oss.c
@@ -29,8 +29,6 @@
 #include <linux/utsname.h>
 #include <linux/mutex.h>
 
-#if defined(CONFIG_SND_OSSEMUL) && defined(CONFIG_PROC_FS)
-
 /*
  *  OSS compatible part
  */
@@ -134,5 +132,3 @@ int snd_info_minor_unregister(void)
 	snd_sndstat_proc_entry = NULL;
 	return 0;
 }
-
-#endif /* CONFIG_SND_OSSEMUL */
diff --git a/sound/core/seq/Makefile b/sound/core/seq/Makefile
index 941f64a853eb..b29ffe835205 100644
--- a/sound/core/seq/Makefile
+++ b/sound/core/seq/Makefile
@@ -6,7 +6,8 @@
 snd-seq-device-objs := seq_device.o
 snd-seq-objs := seq.o seq_lock.o seq_clientmgr.o seq_memory.o seq_queue.o \
                 seq_fifo.o seq_prioq.o seq_timer.o \
-                seq_system.o seq_ports.o seq_info.o
+                seq_system.o seq_ports.o
+snd-seq-$(CONFIG_PROC_FS) += seq_info.o
 snd-seq-midi-objs := seq_midi.o
 snd-seq-midi-emul-objs := seq_midi_emul.o
 snd-seq-midi-event-objs := seq_midi_event.o
diff --git a/sound/core/seq/seq_info.c b/sound/core/seq/seq_info.c
index d3c65e780e9e..97015447b9b3 100644
--- a/sound/core/seq/seq_info.c
+++ b/sound/core/seq/seq_info.c
@@ -27,7 +27,6 @@
 #include "seq_clientmgr.h"
 #include "seq_timer.h"
 
-#ifdef CONFIG_PROC_FS
 static struct snd_info_entry *queues_entry;
 static struct snd_info_entry *clients_entry;
 static struct snd_info_entry *timer_entry;
@@ -80,4 +79,3 @@ int __exit snd_seq_info_done(void)
 	free_info_entries();
 	return 0;
 }
-#endif
diff --git a/sound/core/sound_oss.c b/sound/core/sound_oss.c
index 56d2f409f1ef..86e2d91dd375 100644
--- a/sound/core/sound_oss.c
+++ b/sound/core/sound_oss.c
@@ -19,12 +19,6 @@
  *
  */
 
-#ifdef CONFIG_SND_OSSEMUL
-
-#if !IS_ENABLED(CONFIG_SOUND)
-#error "Enable the OSS soundcore multiplexer (CONFIG_SOUND) in the kernel."
-#endif
-
 #include <linux/init.h>
 #include <linux/export.h>
 #include <linux/slab.h>
@@ -266,5 +260,3 @@ int __init snd_minor_info_oss_init(void)
 	return snd_info_register(entry); /* freed in error path */
 }
 #endif /* CONFIG_PROC_FS */
-
-#endif /* CONFIG_SND_OSSEMUL */
-- 
2.3.5

[PATCH 8/8] ALSA: core: Clean up OSS proc file management

[Takashi Iwai]

A few minor cleanups:
- Move the call of snd_info_minor_register() into snd_info_init() so
  that we can call all proc-related stuff in a shot
- Add missing __init prefix to snd_info_minor_register()
- Return an error properly from snd_oss_info_register()
- Drop snd_info_minor_unregister() that is superfluous now

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/info.h  |  4 +---
 sound/core/info.c     |  3 ++-
 sound/core/info_oss.c | 25 +++++++------------------
 sound/core/sound.c    |  2 --
 4 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/include/sound/info.h b/include/sound/info.h
index 3e2fda3c75ee..16269951bafc 100644
--- a/include/sound/info.h
+++ b/include/sound/info.h
@@ -94,10 +94,8 @@ struct snd_info_entry {
 
 #if defined(CONFIG_SND_OSSEMUL) && defined(CONFIG_PROC_FS)
 int snd_info_minor_register(void);
-int snd_info_minor_unregister(void);
 #else
-#define snd_info_minor_register() /* NOP */
-#define snd_info_minor_unregister() /* NOP */
+#define snd_info_minor_register()	0
 #endif
 
 
diff --git a/sound/core/info.c b/sound/core/info.c
index f8bdd9b6f322..c8a413d6cc9b 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -479,7 +479,8 @@ int __init snd_info_init(void)
 	if (snd_info_version_init() < 0 ||
 	    snd_minor_info_init() < 0 ||
 	    snd_minor_info_oss_init() < 0 ||
-	    snd_card_info_init() < 0)
+	    snd_card_info_init() < 0 ||
+	    snd_info_minor_register() < 0)
 		goto error;
 	return 0;
 
diff --git a/sound/core/info_oss.c b/sound/core/info_oss.c
index bd4d2c6233c2..1478c8dfd473 100644
--- a/sound/core/info_oss.c
+++ b/sound/core/info_oss.c
@@ -35,7 +35,6 @@
 
 static DEFINE_MUTEX(strings);
 static char *snd_sndstat_strings[SNDRV_CARDS][SNDRV_OSS_INFO_DEV_COUNT];
-static struct snd_info_entry *snd_sndstat_proc_entry;
 
 int snd_oss_info_register(int dev, int num, char *string)
 {
@@ -110,25 +109,15 @@ static void snd_sndstat_proc_read(struct snd_info_entry *entry,
 	snd_sndstat_show_strings(buffer, "Mixers", SNDRV_OSS_INFO_DEV_MIXERS);
 }
 
-int snd_info_minor_register(void)
+int __init snd_info_minor_register(void)
 {
 	struct snd_info_entry *entry;
 
 	memset(snd_sndstat_strings, 0, sizeof(snd_sndstat_strings));
-	if ((entry = snd_info_create_module_entry(THIS_MODULE, "sndstat", snd_oss_root)) != NULL) {
-		entry->c.text.read = snd_sndstat_proc_read;
-		if (snd_info_register(entry) < 0) {
-			snd_info_free_entry(entry);
-			entry = NULL;
-		}
-	}
-	snd_sndstat_proc_entry = entry;
-	return 0;
-}
-
-int snd_info_minor_unregister(void)
-{
-	snd_info_free_entry(snd_sndstat_proc_entry);
-	snd_sndstat_proc_entry = NULL;
-	return 0;
+	entry = snd_info_create_module_entry(THIS_MODULE, "sndstat",
+					     snd_oss_root);
+	if (!entry)
+		return -ENOMEM;
+	entry->c.text.read = snd_sndstat_proc_read;
+	return snd_info_register(entry); /* freed in error path */
 }
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 8fc402e4ff35..e5d37bd7c226 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -409,7 +409,6 @@ static int __init alsa_sound_init(void)
 		unregister_chrdev(major, "alsa");
 		return -ENOMEM;
 	}
-	snd_info_minor_register();
 #ifndef MODULE
 	pr_info("Advanced Linux Sound Architecture Driver Initialized.\n");
 #endif
@@ -418,7 +417,6 @@ static int __init alsa_sound_init(void)
 
 static void __exit alsa_sound_exit(void)
 {
-	snd_info_minor_unregister();
 	snd_info_done();
 	unregister_chrdev(major, "alsa");
 }
-- 
2.3.5

Re: [PATCH 0/8] Proc file cleanups

[Jaroslav Kysela]

Dne 23.4.2015 v 16:49 Takashi Iwai napsal(a):
> Hi,
> 
> here is a series of patches for cleaning up the ALSA proc file
> management code.  The biggest change is the use of seq_file for text
> reads, but all are internal and the API is kept intact.
> 
> The rest are mostly small cleanups and refactoring.
> 
> The patches are found in topic/proc branch of sound git tree.

The changes look good.

Acked-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.