[pacman-dev] [PATCH] Fix double close of the lock file

[Jonathan Conder]

On 5 February 2011 13:39, Jonathan Conder <jonno.conder at gmail.com> wrote:

> According to FOPEN(3), using fclose on an fdopen'd file stream also
> closes the underlying file descriptor. This happened in _alpm_lckmk
> (util.c), which meant that when alpm_trans_release closed it again, the
> log file (which reused the original file descriptor) was closed instead.
>
> Signed-off-by: Jonathan Conder <jonno.conder at gmail.com>
> ---
>

Could I get at least an ack for this please? I sent it a week ago and
haven't heard anything.


>  lib/libalpm/handle.c |    1 -
>  lib/libalpm/handle.h |    2 +-
>  lib/libalpm/trans.c  |   13 +++++--------
>  lib/libalpm/util.c   |    7 +++----
>  lib/libalpm/util.h   |    2 +-
>  5 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
> index aa34cf4..8f44e94 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -49,7 +49,6 @@ pmhandle_t *_alpm_handle_new()
>        ALPM_LOG_FUNC;
>
>        CALLOC(handle, 1, sizeof(pmhandle_t), RET_ERR(PM_ERR_MEMORY, NULL));
> -       handle->lckfd = -1;
>
>        return(handle);
>  }
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index 1834994..6884666 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -34,7 +34,7 @@ typedef struct _pmhandle_t {
>        pmdb_t *db_local;       /* local db pointer */
>        alpm_list_t *dbs_sync;  /* List of (pmdb_t *) */
>        FILE *logstream;        /* log file stream pointer */
> -       int lckfd;              /* lock file descriptor if one exists */
> +       FILE *lckstream;        /* lock file stream pointer if one exists
> */
>        pmtrans_t *trans;
>
>        /* callback functions */
> diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
> index 02612ec..e13a7b2 100644
> --- a/lib/libalpm/trans.c
> +++ b/lib/libalpm/trans.c
> @@ -73,8 +73,8 @@ int SYMEXPORT alpm_trans_init(pmtransflag_t flags,
>
>        /* lock db */
>        if(!(flags & PM_TRANS_FLAG_NOLOCK)) {
> -               handle->lckfd = _alpm_lckmk();
> -               if(handle->lckfd == -1) {
> +               handle->lckstream = _alpm_lckmk();
> +               if(handle->lckstream == NULL) {
>                        RET_ERR(PM_ERR_HANDLE_LOCK, -1);
>                }
>        }
> @@ -260,12 +260,9 @@ int SYMEXPORT alpm_trans_release()
>
>        /* unlock db */
>        if(!nolock_flag) {
> -               if(handle->lckfd != -1) {
> -                       int fd;
> -                       do {
> -                               fd = close(handle->lckfd);
> -                       } while(fd == -1 && errno == EINTR);
> -                       handle->lckfd = -1;
> +               if(handle->lckstream != NULL) {
> +                       fclose(handle->lckstream);
> +                       handle->lckstream = NULL;
>                }
>                if(_alpm_lckrm()) {
>                        _alpm_log(PM_LOG_WARNING, _("could not remove lock
> file %s\n"),
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 32eaa44..4d152ee 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -197,7 +197,7 @@ char *_alpm_strtrim(char *str)
>  }
>
>  /* Create a lock file */
> -int _alpm_lckmk()
> +FILE *_alpm_lckmk()
>  {
>        int fd;
>        char *dir, *ptr;
> @@ -220,10 +220,9 @@ int _alpm_lckmk()
>                fprintf(f, "%ld\n", (long)getpid());
>                fflush(f);
>                fsync(fd);
> -               fclose(f);
> -               return(fd);
> +               return(f);
>        }
> -       return(-1);
> +       return(NULL);
>  }
>
>  /* Remove a lock file */
> diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
> index 8a3154a..edd51d5 100644
> --- a/lib/libalpm/util.h
> +++ b/lib/libalpm/util.h
> @@ -62,7 +62,7 @@ int _alpm_makepath(const char *path);
>  int _alpm_makepath_mode(const char *path, mode_t mode);
>  int _alpm_copyfile(const char *src, const char *dest);
>  char *_alpm_strtrim(char *str);
> -int _alpm_lckmk();
> +FILE *_alpm_lckmk();
>  int _alpm_lckrm();
>  int _alpm_unpack_single(const char *archive, const char *prefix, const
> char *fn);
>  int _alpm_unpack(const char *archive, const char *prefix, alpm_list_t
> *list, int breakfirst);
> --
> 1.7.4
>
>