Re: [PATCH 1/2] compat/win32/pthread: add pthread_once()
- From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
- Date
- Jun 26, 2020, 14:13 UTC
- Message-ID
- <CAHd-oW7Ev_4=Kw8z=qi6U_oR6a7dzhL3kTH=xkL1PFsfqXu5QQ@mail.gmail.com>
- In-Reply-To
- <CAP8UFD3NwKtWYL_H5fWjT-HQVd3W8ZJ49p+faDyMQ+==ttCWjA@mail.gmail.com>
On Fri, Jun 26, 2020 at 2:45 AM Christian Couder <christian.couder@gmail.com> wrote:
Show 9 quoted lines
> > 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!
Show 10 quoted lines
> > 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.
Show 31 quoted lines
> > 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!