[pacman-dev] [PATCHv3] alpm_handle: store lock file descriptor

[Allan McRae]

On 27/10/13 01:32, Andrew Gregory wrote:
> There was a brief window between opening the file descriptor and
> creating a stream to it.  If the process was interrupted during that
> window the lock file would not be removed correctly.
> 
> The pid is no longer printed to the lock file as this was virtually
> meaningless for lock files on NFS.
> 
> Fixes FS#35603
> 
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---

Can someone else also look at commit acd92694 and ensure we are not
regressing here.   I think this is fine...


> 
> Initialize lockfd = -1 because using stdin as the lock is bad...
> 
>  lib/libalpm/handle.c | 25 +++++++++----------------
>  lib/libalpm/handle.h |  2 +-
>  2 files changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
> index 53c86c5..4871e1e 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -44,6 +44,7 @@ alpm_handle_t *_alpm_handle_new(void)
>  
>  	CALLOC(handle, 1, sizeof(alpm_handle_t), return NULL);
>  	handle->deltaratio = 0.0;
> +	handle->lockfd = -1;
>  
>  	return handle;
>  }
> @@ -91,11 +92,10 @@ void _alpm_handle_free(alpm_handle_t *handle)
>  /** Lock the database */
>  int _alpm_handle_lock(alpm_handle_t *handle)
>  {
> -	int fd;
>  	char *dir, *ptr;
>  
>  	ASSERT(handle->lockfile != NULL, return -1);
> -	ASSERT(handle->lckstream == NULL, return 0);
> +	ASSERT(handle->lockfd < 0, return 0);
>  
>  	/* create the dir of the lockfile first */
>  	dir = strdup(handle->lockfile);
> @@ -110,27 +110,20 @@ int _alpm_handle_lock(alpm_handle_t *handle)
>  	FREE(dir);
>  
>  	do {
> -		fd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000);
> -	} while(fd == -1 && errno == EINTR);
> -	if(fd >= 0) {
> -		FILE *f = fdopen(fd, "w");
> -		fprintf(f, "%ld\n", (long)getpid());
> -		fflush(f);
> -		fsync(fd);
> -		handle->lckstream = f;
> -		return 0;
> -	}
> -	return -1;
> +		handle->lockfd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000);
> +	} while(handle->lockfd == -1 && errno == EINTR);
> +
> +	return (handle->lockfd >= 0 ? 0 : -1);
>  }
>  
>  /** Remove a lock file */
>  int _alpm_handle_unlock(alpm_handle_t *handle)
>  {
>  	ASSERT(handle->lockfile != NULL, return -1);
> -	ASSERT(handle->lckstream != NULL, return 0);
> +	ASSERT(handle->lockfd >= 0, return 0);
>  
> -	fclose(handle->lckstream);
> -	handle->lckstream = NULL;
> +	close(handle->lockfd);
> +	handle->lockfd = -1;
>  
>  	if(unlink(handle->lockfile) && errno != ENOENT) {
>  		return -1;
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index 5e84d58..a4e2cf7 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -55,7 +55,7 @@ struct __alpm_handle_t {
>  	alpm_db_t *db_local;       /* local db pointer */
>  	alpm_list_t *dbs_sync;  /* List of (alpm_db_t *) */
>  	FILE *logstream;        /* log file stream pointer */
> -	FILE *lckstream;        /* lock file stream pointer if one exists */
> +	int lockfd;             /* lock file descriptor */
>  	alpm_trans_t *trans;
>  
>  #ifdef HAVE_LIBCURL
>