Re: [PATCH 1/2] compat/win32/pthread: add pthread_once()

[Matheus Tavares Bernardino <matheus.bernardino@usp.br>]

On Fri, Jun 26, 2020 at 2:45 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 3:00 AM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
> >
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> > ---
>
> The commit message might want to explain a bit the purpose of adding
> these features.

Will do!

> > Note: the pthread_once() function is adapted from:
> > https://git.libav.org/?p=libav.git;a=commitdiff;h=b22693b06d1e5d73454a65c203b4d31c1ca5b69a
> >
> > Which is LGPLv2.1. Should I add any notice/acknowledgment somewhere,
> > besides the comment I added above the function?
>
> Yeah, I think you should also tell in the commit message where the
> code comes from (along with the hash of the commit) and that libav is
> LGPLv2.1 which is compatible with GPLv2 as explained in section 3 of
> the LGPLv2.1.

Sounds good, I'll add that information.

> >  compat/win32/pthread.c | 22 ++++++++++++++++++++++
> >  compat/win32/pthread.h |  5 +++++
> >  thread-utils.c         | 11 +++++++++++
> >  thread-utils.h         |  6 ++++++
> >  4 files changed, 44 insertions(+)
> >
> > diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> > index 2e7eead42c..5a7ecbd999 100644
> > --- a/compat/win32/pthread.c
> > +++ b/compat/win32/pthread.c
> > @@ -56,3 +56,25 @@ pthread_t pthread_self(void)
> >         t.tid = GetCurrentThreadId();
> >         return t;
> >  }
> > +
> > +/* Adapted from libav's compat/w32pthreads.h. */
> > +int pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
> > +{
> > +       BOOL pending = FALSE;
> > +       int ret = 0;
> > +
> > +       if(!InitOnceBeginInitialize(once_control, 0, &pending, NULL)) {
>
> We put a space between "if" and the following "(". It might also be
> interesting to know perhaps in the commit message how much you adapted
> the code.
>
> For example perhaps a good strategy would be in the commit that
> imports the code to do the minimal amount of change so that it builds
> and passes the test, and then to have another commit that adapts the
> style of the code.

Makes sense. Alternatively, since the code ported from libav is quite
small, I think we might as well already adapt the style in this same
commit. I'll fix that for v2, thanks!