Anyone using alsactl preinit / postinit scripts?

Anyone using alsactl preinit / postinit scripts?

[Takashi Iwai]

Hi,

is anyone already using preinit / postinit scripts of alsactl init?
I'm thinking of appliny the patch below, but before that, I'd like to
make sure that *.conf is the appropriate extension.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] alsactl: Read only *.conf files when a directory is passed via INCLUDE

When alsactl init is invoked and a directory path is passed to INCLUDE
command in the config file, read only *.conf files in that directory.
This will avoid reading backup files or invalid files that have been
created accidentally.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 alsactl/init_parse.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/alsactl/init_parse.c b/alsactl/init_parse.c
index 51b515c..83d12ba 100644
--- a/alsactl/init_parse.c
+++ b/alsactl/init_parse.c
@@ -1502,12 +1502,16 @@ static int parse_line(struct space *space, char *line, size_t linesize)
 			if (dir) {
 				count = strlen(string);
 				while ((dirent = readdir(dir)) != NULL) {
-					if (strcmp(dirent->d_name, ".") == 0 ||
-					    strcmp(dirent->d_name, "..") == 0)
+					const char *name = dirent->d_name;
+					const char *ext;
+					if (*name == '.')
+						continue;
+					ext = strrchr(name, ',');
+					if (!ext || strcmp(ext, ".conf"))
 						continue;
 					string[count] = '\0';
 					strlcat(string, "/", sizeof(string));
-					strlcat(string, dirent->d_name, sizeof(string));
+					strlcat(string, name, sizeof(string));
 					space->go_to = NULL;
 					space->rootdir = new_root_dir(string);
 					if (space->rootdir) {
-- 
1.7.10.3

Re: Anyone using alsactl preinit / postinit scripts?

[Jaroslav Kysela]

Date 14.6.2012 14:10, Takashi Iwai wrote:
> Hi,
> 
> is anyone already using preinit / postinit scripts of alsactl init?
> I'm thinking of appliny the patch below, but before that, I'd like to
> make sure that *.conf is the appropriate extension.

Hi,

This filename filter looks good to me. I have no objections to use the
.conf extension.

While improving this code, adding a filename sort might be also useful ;-)

> ---
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] alsactl: Read only *.conf files when a directory is passed via INCLUDE
> 
> When alsactl init is invoked and a directory path is passed to INCLUDE
> command in the config file, read only *.conf files in that directory.
> This will avoid reading backup files or invalid files that have been
> created accidentally.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  alsactl/init_parse.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/alsactl/init_parse.c b/alsactl/init_parse.c
> index 51b515c..83d12ba 100644
> --- a/alsactl/init_parse.c
> +++ b/alsactl/init_parse.c
> @@ -1502,12 +1502,16 @@ static int parse_line(struct space *space, char *line, size_t linesize)
>  			if (dir) {
>  				count = strlen(string);
>  				while ((dirent = readdir(dir)) != NULL) {
> -					if (strcmp(dirent->d_name, ".") == 0 ||
> -					    strcmp(dirent->d_name, "..") == 0)
> +					const char *name = dirent->d_name;
> +					const char *ext;
> +					if (*name == '.')
> +						continue;
> +					ext = strrchr(name, ',');

'.' should be matched here...

> +					if (!ext || strcmp(ext, ".conf"))
>  						continue;

Please, update also the documentation in alsactl_init.xml .

				Thanks,
					Jaroslav

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

Re: Anyone using alsactl preinit / postinit scripts?

[Takashi Iwai]

At Thu, 14 Jun 2012 15:07:40 +0200,
Jaroslav Kysela wrote:
> 
> Date 14.6.2012 14:10, Takashi Iwai wrote:
> > Hi,
> > 
> > is anyone already using preinit / postinit scripts of alsactl init?
> > I'm thinking of appliny the patch below, but before that, I'd like to
> > make sure that *.conf is the appropriate extension.
> 
> Hi,
> 
> This filename filter looks good to me. I have no objections to use the
> .conf extension.
> 
> While improving this code, adding a filename sort might be also useful ;-)

Ah, that's definitely good.  I thought of some number prefix check,
but I totally forgot the fact it's not sorted.

The revised patch using scandir() is below.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] alsactl: Read only *.conf files when a directory is passed
 via INCLUDE

When alsactl init is invoked and a directory path is passed to INCLUDE
command in the config file, read only *.conf files in that directory.
This will avoid reading backup files or invalid files that have been
created accidentally.

Also by using scandir() with alphasort(), alsactl reads the files in
alphabetical order.  Thus it's highly recommended to use some number
prefix to the file name for assuring the order.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 alsactl/alsactl_init.xml |   10 +++++++++-
 alsactl/init_parse.c     |   35 +++++++++++++++++++++++++----------
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/alsactl/alsactl_init.xml b/alsactl/alsactl_init.xml
index eefe9ef..bce26f5 100644
--- a/alsactl/alsactl_init.xml
+++ b/alsactl/alsactl_init.xml
@@ -474,7 +474,15 @@
             <varlistentry>
               <term><option>INCLUDE</option></term>
               <listitem>
-                <para>Include specified filename or all files in specified directory</para>
+                <para>Include the specified filename or files in specified directory.
+		</para>
+		<para>
+		When a directory is specified, only the files with the
+		extension ".conf" are read.
+		Also they are read in the alphabetical order.
+		Thus it's highly recommended to use some number prefix
+		(e.g. "01-something.conf") to assure the order of execucions.
+		</para>
               </listitem>
             </varlistentry>
 
diff --git a/alsactl/init_parse.c b/alsactl/init_parse.c
index 51b515c..8a7173b 100644
--- a/alsactl/init_parse.c
+++ b/alsactl/init_parse.c
@@ -1278,6 +1278,13 @@ static char *new_root_dir(const char *filename)
 	return res;
 }
 
+/* return non-zero if the file name has the extension ".conf" */
+static int conf_name_filter(const struct dirent *d)
+{
+	char *ext = strrchr(d->d_name, '.');
+	return ext && !strcmp(ext, ".conf");
+}
+
 static int parse_line(struct space *space, char *line, size_t linesize)
 {
 	char *linepos;
@@ -1480,8 +1487,7 @@ static int parse_line(struct space *space, char *line, size_t linesize)
 		if (strcasecmp(key, "INCLUDE") == 0) {
 			char *rootdir, *go_to;
 			const char *filename;
-			struct dirent *dirent;
-			DIR *dir;
+			struct stat st;
 			int linenum;
 			if (op != KEY_OP_ASSIGN) {
 				Perror(space, "invalid INCLUDE operation");
@@ -1498,18 +1504,27 @@ static int parse_line(struct space *space, char *line, size_t linesize)
 			go_to = space->go_to;
 			filename = space->filename;
 			linenum = space->linenum;
-			dir = opendir(string);
-			if (dir) {
+			if (stat(string, &st)) {
+				Perror(space, "invalid filename '%s'", string);
+				continue;
+			}
+			if (S_ISDIR(st.st_mode)) {
+				struct dirent **list;
+				int i, num;
+				num = scandir(string, &list, conf_name_filter,
+					      alphasort);
+				if (num < 0) {
+					Perror(space, "invalid directory '%s'", string);
+					continue;
+				}
 				count = strlen(string);
-				while ((dirent = readdir(dir)) != NULL) {
-					if (strcmp(dirent->d_name, ".") == 0 ||
-					    strcmp(dirent->d_name, "..") == 0)
-						continue;
+				for (i = 0; i < num; i++) {
 					string[count] = '\0';
 					strlcat(string, "/", sizeof(string));
-					strlcat(string, dirent->d_name, sizeof(string));
+					strlcat(string, list[i]->d_name, sizeof(string));
 					space->go_to = NULL;
 					space->rootdir = new_root_dir(string);
+					free(list[i]);
 					if (space->rootdir) {
 						err = parse(space, string);
 						free(space->rootdir);
@@ -1522,7 +1537,7 @@ static int parse_line(struct space *space, char *line, size_t linesize)
 					if (err)
 						break;
 				}
-				closedir(dir);
+				free(list);
 			} else {
 				space->go_to = NULL;
 				space->rootdir = new_root_dir(string);
-- 
1.7.10.3