<p dir="ltr"><br>
On Apr 11, 2016 08:28, "Thomas Guillem" <<a href="mailto:thomas@gllm.fr">thomas@gllm.fr</a>> wrote:<br>
><br>
> Hi,</p>
<p dir="ltr">Hi Thomas,</p>
<p dir="ltr">First of all, thank you for the review! See my responses inline.</p>
<p dir="ltr">><br>
> On Mon, Apr 11, 2016, at 02:44, Sean McGovern wrote:<br>
> > Additionally, remove the unlock call as fclose() will drop<br>
> > the lock for us.<br>
> > ---<br>
> >  modules/keystore/file.c |   37 +++++++++++++++++++++++++++++--------<br>
> >  1 file changed, 29 insertions(+), 8 deletions(-)<br>
> ><br>
> > diff --git a/modules/keystore/file.c b/modules/keystore/file.c<br>
> > index 381ec87..ec166db 100644<br>
> > --- a/modules/keystore/file.c<br>
> > +++ b/modules/keystore/file.c<br>
> > @@ -27,6 +27,9 @@<br>
> >  #ifdef HAVE_FLOCK<br>
> >  #include <sys/file.h><br>
> >  #endif<br>
> > +#ifdef HAVE_FCNTL<br>
> elif?<br>
> > +#include <fcntl.h><br>
> > +#endif<br>
> ><br>
> >  #include <vlc_common.h><br>
> >  #include <vlc_plugin.h><br>
> > @@ -263,6 +266,24 @@ end:<br>
> >      return VLC_SUCCESS;<br>
> >  }<br>
> ><br>
> > +#if (defined (HAVE_FCNTL) && defined (F_SETLKW))<br>
><br>
> maybe:<br>
> #if (!defined(HAVE_FLOCK) && defined (HAVE_FCNTL) && defined (F_SETLKW))<br>
></p>
<p dir="ltr">Sure, no problem.</p>
<p dir="ltr">><br>
> > +static int<br>
> > +posix_lock_fd(int fd)<br>
> > +{<br>
> > +    struct flock lock;<br>
> > +<br>
> > +    if(fd == -1)<br>
> > +        return -1;<br>
> > +<br>
> > +    lock.l_start = 0;<br>
> > +    lock.l_len = 0;<br>
> > +    lock.l_whence = SEEK_SET;<br>
> > +    lock.l_type = F_WRLCK;<br>
> > +<br>
> > +    return fcntl(fd, F_SETLKW, &lock);<br>
> > +}<br>
> > +#endif<br>
> > +<br>
> >  static int<br>
> >  file_open(const char *psz_file, const char *psz_mode, FILE **pp_file)<br>
> >  {<br>
> > @@ -276,20 +297,20 @@ file_open(const char *psz_file, const char<br>
> > *psz_mode, FILE **pp_file)<br>
> ><br>
> >  #ifdef HAVE_FLOCK<br>
> >      if (flock(i_fd, LOCK_EX) != 0)<br>
> > +#else<br>
> > +    if (posix_lock_fd(i_fd) != 0)<br>
> > +#endif<br>
> >      {<br>
> >          fclose(p_file);<br>
> >      }<br>
> > -#endif<br>
> > +<br>
><br>
> Why moving this ending ?<br>
> On windows, there is no lock fonction, but opening the file in rw is<br>
> enough to lock it.<br>
></p>
<p dir="ltr">Oh, woops! Good catch -- will fix and re-send later today.</p>
<p dir="ltr">> >      *pp_file = p_file;<br>
> >      return i_fd;<br>
> >  }<br>
> ><br>
> >  static void<br>
> > -file_close(FILE *p_file, int i_fd)<br>
> > +file_close(FILE *p_file)<br>
> >  {<br>
> > -#ifdef HAVE_FLOCK<br>
> > -    flock(i_fd, LOCK_UN);<br>
> > -#endif<br>
> >      fclose(p_file);<br>
> >  }<br>
> ><br>
> > @@ -349,7 +370,7 @@ Store(vlc_keystore *p_keystore, const char *const<br>
> > ppsz_values[KEY_MAX],<br>
> >      i_ret = file_save(p_keystore, p_file, i_fd, &list);<br>
> ><br>
> >  end:<br>
> > -    file_close(p_file, i_fd);<br>
> > +    file_close(p_file);<br>
> >      ks_list_free(&list);<br>
> >      return i_ret;<br>
> >  }<br>
> > @@ -413,7 +434,7 @@ Find(vlc_keystore *p_keystore, const char *const<br>
> > ppsz_values[KEY_MAX],<br>
> ><br>
> >      *pp_entries = out_list.p_entries;<br>
> >  end:<br>
> > -    file_close(p_file, i_fd);<br>
> > +    file_close(p_file);<br>
> >      ks_list_free(&list);<br>
> >      return out_list.i_count;<br>
> >  }<br>
> > @@ -445,7 +466,7 @@ Remove(vlc_keystore *p_keystore, const char *const<br>
> > ppsz_values[KEY_MAX])<br>
> >          i_count = 0;<br>
> ><br>
> >  end:<br>
> > -    file_close(p_file, i_fd);<br>
> > +    file_close(p_file);<br>
> >      ks_list_free(&list);<br>
> >      return i_count;<br>
> >  }<br>
> > --<br>
> > </p>
<p dir="ltr">-- Sean McG.</p>