[vlc-devel] [PATCH v2] keystore/file: use POSIX file locking when flock() is unavailable

[Sean McGovern]

On Apr 11, 2016 19:12, "Sean McGovern" <gseanmcg at gmail.com> wrote:
>
> Additionally, remove the unlock call as fclose() will drop
> the lock for us.
> ---
>  modules/keystore/file.c |   39 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/modules/keystore/file.c b/modules/keystore/file.c
> index 381ec87..25eddd9 100644
> --- a/modules/keystore/file.c
> +++ b/modules/keystore/file.c
> @@ -27,6 +27,9 @@
>  #ifdef HAVE_FLOCK
>  #include <sys/file.h>
>  #endif
> +#ifdef HAVE_FCNTL
> +#include <fcntl.h>
> +#endif
>
>  #include <vlc_common.h>
>  #include <vlc_plugin.h>
> @@ -262,6 +265,24 @@ end:
>      }
>      return VLC_SUCCESS;
>  }
> +
> +#if (!defined(HAVE_FLOCK) && defined (HAVE_FCNTL) && defined (F_SETLKW))
> +static int
> +posix_lock_fd(int fd)
> +{
> +    struct flock lock;
> +
> +    if(fd == -1)
> +        return -1;
> +
> +    lock.l_start = 0;
> +    lock.l_len = 0;
> +    lock.l_whence = SEEK_SET;
> +    lock.l_type = F_RDLCK;
> +
> +    return fcntl(fd, F_SETLKW, &lock);
> +}
> +#endif
>
>  static int
>  file_open(const char *psz_file, const char *psz_mode, FILE **pp_file)
> @@ -279,17 +300,21 @@ file_open(const char *psz_file, const char
*psz_mode, FILE **pp_file)
>      {
>          fclose(p_file);
>      }
> +#else
> +#if defined (HAVE_FCNTL) && defined (F_SETLKW)
> +    if (posix_lock_fd(i_fd) != 0)
> +    {
> +        fclose(p_file);
> +    }
> +#endif
>  #endif
>      *pp_file = p_file;
>      return i_fd;
>  }
>
>  static void
> -file_close(FILE *p_file, int i_fd)
> +file_close(FILE *p_file)
>  {
> -#ifdef HAVE_FLOCK
> -    flock(i_fd, LOCK_UN);
> -#endif
>      fclose(p_file);
>  }
>
> @@ -349,7 +374,7 @@ Store(vlc_keystore *p_keystore, const char *const
ppsz_values[KEY_MAX],
>      i_ret = file_save(p_keystore, p_file, i_fd, &list);
>
>  end:
> -    file_close(p_file, i_fd);
> +    file_close(p_file);
>      ks_list_free(&list);
>      return i_ret;
>  }
> @@ -413,7 +438,7 @@ Find(vlc_keystore *p_keystore, const char *const
ppsz_values[KEY_MAX],
>
>      *pp_entries = out_list.p_entries;
>  end:
> -    file_close(p_file, i_fd);
> +    file_close(p_file);
>      ks_list_free(&list);
>      return out_list.i_count;
>  }
> @@ -445,7 +470,7 @@ Remove(vlc_keystore *p_keystore, const char *const
ppsz_values[KEY_MAX])
>          i_count = 0;
>
>  end:
> -    file_close(p_file, i_fd);
> +    file_close(p_file);
>      ks_list_free(&list);
>      return i_count;
>  }
> --
>

Actually -- this module needs to lock for both reading _and_ writing, yes?

In which case this patch is wrong.

Will poke at it tomorrow with a clearer head.

-- Sean McG.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160411/05753585/attachment.html>