[alsa-devel] [PATCH v2] alsactl: Store lockfile in /tmp

[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?

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.

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


Takashi

> 
> Signed-off-by: Julian Scheel <julian at 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 at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>