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

[Sean McGovern]

On Apr 11, 2016 08:28, "Thomas Guillem" <thomas at gllm.fr> wrote:
>
> Hi,

Hi Thomas,

First of all, thank you for the review! See my responses inline.

>
> On Mon, Apr 11, 2016, at 02:44, Sean McGovern wrote:
> > Additionally, remove the unlock call as fclose() will drop
> > the lock for us.
> > ---
> >  modules/keystore/file.c |   37 +++++++++++++++++++++++++++++--------
> >  1 file changed, 29 insertions(+), 8 deletions(-)
> >
> > diff --git a/modules/keystore/file.c b/modules/keystore/file.c
> > index 381ec87..ec166db 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
> elif?
> > +#include <fcntl.h>
> > +#endif
> >
> >  #include <vlc_common.h>
> >  #include <vlc_plugin.h>
> > @@ -263,6 +266,24 @@ end:
> >      return VLC_SUCCESS;
> >  }
> >
> > +#if (defined (HAVE_FCNTL) && defined (F_SETLKW))
>
> maybe:
> #if (!defined(HAVE_FLOCK) && defined (HAVE_FCNTL) && defined (F_SETLKW))
>

Sure, no problem.

>
> > +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_WRLCK;
> > +
> > +    return fcntl(fd, F_SETLKW, &lock);
> > +}
> > +#endif
> > +
> >  static int
> >  file_open(const char *psz_file, const char *psz_mode, FILE **pp_file)
> >  {
> > @@ -276,20 +297,20 @@ file_open(const char *psz_file, const char
> > *psz_mode, FILE **pp_file)
> >
> >  #ifdef HAVE_FLOCK
> >      if (flock(i_fd, LOCK_EX) != 0)
> > +#else
> > +    if (posix_lock_fd(i_fd) != 0)
> > +#endif
> >      {
> >          fclose(p_file);
> >      }
> > -#endif
> > +
>
> Why moving this ending ?
> On windows, there is no lock fonction, but opening the file in rw is
> enough to lock it.
>

Oh, woops! Good catch -- will fix and re-send later today.

> >      *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 +370,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 +434,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 +466,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;
> >  }
> > --
> >

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