At Wed, 07 May 2014 10:49:59 +0200, Julian Scheel wrote:

Am 07.05.2014 09:19, schrieb Takashi Iwai:

...

At Tue, 6 May 2014 21:32:19 +0200, Julian Scheel wrote:

...

It can not be generally assumed that the directories in which asound.state resides are writable. Instead using /tmp as location for lock files seems more reliable.

The subject and changelog don't match with the actual change. Now it's /var/log instead of /tmp, right?

Sorry for that. Must have been too late.

Seems so...

...

Besides that, it'd be better to allow a full path name for a lock file instead of a directory name. If you give a different file name via -f option, you have a high chance to conflict with the existing file in /var/lock.

So, you'd prefer a --lock-file/-L option which can be used to set an explicit lock file?

Yes.

...

Furthermore, for solving *your* problem (restoring from read-only rootfs), an easier option would be allowing to restore the system default without locking.

While this is true I think making the locking mechanism more robust is a good thing anyway.

Of course, it's good to improve the lock mechanism. OTOH, adding an option like --no-lock would be good, too. This is a different solution, and the idea doesn't conflict.

Takashi

Julian

...

Takashi

...

Signed-off-by: Julian Scheel julian@jusst.de

alsactl/alsactl.c | 7 +++++++ alsactl/alsactl.h | 1 + alsactl/lock.c | 13 ++++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/alsactl/alsactl.c b/alsactl/alsactl.c index 6bc013f..415dfb8 100644 --- a/alsactl/alsactl.c +++ b/alsactl/alsactl.c @@ -38,6 +38,9 @@ #ifndef SYS_PIDFILE #define SYS_PIDFILE "/var/run/alsactl.pid" #endif +#ifndef SYS_LOCKPATH +#define SYS_LOCKPATH "/var/lock" +#endif

int debugflag = 0; int force_restore = 1; @@ -46,6 +49,7 @@ int do_lock = 0; int use_syslog = 0; char *command; char *statefile = NULL; +char *lockpath = SYS_LOCKPATH;

#define TITLE 0x0100 #define HEADER 0x0200 @@ -71,6 +75,7 @@ static struct arg args[] = { { HEADER, NULL, "Available state options:" }, { FILEARG | 'f', "file", "configuration file (default " SYS_ASOUNDRC ")" }, { 'l', "lock", "use file locking to serialize concurrent access" }, +{ FILEARG | 'D', "lock-dir", "directory to use for lock files (default " SYS_LOCKPATH ")" }, { 'F', "force", "try to restore the matching controls as much as possible" }, { 0, NULL, " (default mode)" }, { 'g', "ignore", "ignore 'No soundcards found' error" }, @@ -232,6 +237,8 @@ int main(int argc, char *argv[]) case 'l': do_lock = 1; break;

• case 'D':
  

• 	lockpath = optarg;
  

  case 'F': force_restore = 1; break;

diff --git a/alsactl/alsactl.h b/alsactl/alsactl.h index 9109a70..6c6bee5 100644 --- a/alsactl/alsactl.h +++ b/alsactl/alsactl.h @@ -5,6 +5,7 @@ extern int do_lock; extern int use_syslog; extern char *command; extern char *statefile; +extern char *lockpath;

void info_(const char *fcn, long line, const char *fmt, ...); void error_(const char *fcn, long line, const char *fmt, ...); diff --git a/alsactl/lock.c b/alsactl/lock.c index 587a109..c69e285 100644 --- a/alsactl/lock.c +++ b/alsactl/lock.c @@ -36,17 +36,24 @@ static int state_lock_(const char *file, int lock, int timeout) struct flock lck; struct stat st; char lcktxt[12];

• char *filename; char *nfile;

  if (!do_lock) return 0;

• nfile = malloc(strlen(file) + 6);

• /* only use the actual filename, not the path */
• filename = strrchr(file, '/');
• if (!filename)

• filename = file;
  

• nfile = malloc(strlen(lockpath) + strlen(filename) + 7); if (nfile == NULL) { error("No enough memory..."); return -ENOMEM; }

• strcpy(nfile, file);
• strcat(nfile, ".lock");

• sprintf(nfile, "%s/%s.lock", lockpath, filename); lck.l_type = lock ? F_WRLCK : F_UNLCK; lck.l_whence = SEEK_SET; lck.l_start = 0;

-- 1.9.2

---

Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel