Re: [PATCH] alsactl: Store lockfile in /tmp

[Takashi Iwai <tiwai@suse.de>]

At Tue, 06 May 2014 17:00:47 +0200,
Julian Scheel wrote:
> 
> On 06.05.2014 16:53, Jaroslav Kysela wrote:
> > Date 6.5.2014 13:57, 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.
> > Apart the missing free for the mallocated string and ommiting the TMPDIR
> > environment variable, I think that the right directory for global locks
> > is /var/lock . The default asound.state directory is now /var/lib/alsa -
> > I don't see the benefit.
> 
> The patch does not allocate anything that was not allocated before.
> nfile was allocated before and is freed a few lines after the patch 
> content. filename is just a pointer, not a newly allocated buffer.
> Using /var/lock instead of /tmp sounds sane, yes.
> 
> > What's the reason for this change? Perhaps using an environmental
> > variable to override the lock path may be more appropriate for a custom
> > directory structure.
> 
> We're running alsactl restore on startup of an embedded system which 
> uses a read-only rootfs. So it can't create the lockfile in the default 
> place and hence will not restore anything.

OK, if so, /var/lock would be a better option for the default
asound.state.  For other cases, we can add an option to pass the lock
file path.


thanks,

Takashi

> 
> -Julian
> 
> >                                     Jaroslav
> >
> >>
> >> Signed-off-by: Julian Scheel <julian@jusst.de>
> >> ---
> >>   alsactl/lock.c | 13 ++++++++++---
> >>   1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/alsactl/lock.c b/alsactl/lock.c
> >> index 587a109..7ca3a09 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(filename) + 10);
> >>   	if (nfile == NULL) {
> >>   		error("No enough memory...");
> >>   		return -ENOMEM;
> >>   	}
> >> -	strcpy(nfile, file);
> >> -	strcat(nfile, ".lock");
> >> +
> >> +	sprintf(nfile, "/tmp/%s.lock", filename);
> >>   	lck.l_type = lock ? F_WRLCK : F_UNLCK;
> >>   	lck.l_whence = SEEK_SET;
> >>   	lck.l_start = 0;
> >>
> >
> >
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>