| Message ID | e5a10d3f2152859b75bd815c37511975057d0ab0.1593115455.git.matheus.bernardino@usp.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make oid_to_hex() thread-safe | expand |
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. > 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. > 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. > + ret = err_win_to_posix(GetLastError()); > + goto out; > + } > + > + if (pending) > + init_routine(); > + > + if(!InitOnceComplete(once_control, 0, NULL)) Space missing between "if" and the following "(". > + ret = err_win_to_posix(GetLastError()); > + > +out: > + /* POSIX doesn't allow pthread_once() to return EINTR */ > + return ret == EINTR ? EIO : ret; > +}
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!
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)) { + ret = err_win_to_posix(GetLastError()); + goto out; + } + + if (pending) + init_routine(); + + if(!InitOnceComplete(once_control, 0, NULL)) + ret = err_win_to_posix(GetLastError()); + +out: + /* POSIX doesn't allow pthread_once() to return EINTR */ + return ret == EINTR ? EIO : ret; +} diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 737983d00b..c50f1e89c7 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -40,6 +40,11 @@ typedef int pthread_mutexattr_t; #define pthread_cond_signal WakeConditionVariable #define pthread_cond_broadcast WakeAllConditionVariable +#define pthread_once_t INIT_ONCE + +#define PTHREAD_ONCE_INIT INIT_ONCE_STATIC_INIT +int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)); + /* * Simple thread creation implementation using pthread API */ diff --git a/thread-utils.c b/thread-utils.c index 5329845691..937deb3f2e 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -122,4 +122,15 @@ int dummy_pthread_join(pthread_t pthread, void **retval) return ENOSYS; } +int nothreads_pthread_once(pthread_once_t *once_control, + void (*init_routine)(void)) +{ + if (*once_control == 1) + return 0; + + init_routine(); + *once_control = 1; + return 0; +} + #endif diff --git a/thread-utils.h b/thread-utils.h index 4961487ed9..bab9dc5e4d 100644 --- a/thread-utils.h +++ b/thread-utils.h @@ -19,6 +19,7 @@ #define pthread_mutex_t int #define pthread_cond_t int #define pthread_key_t int +#define pthread_once_t int #define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex) #define pthread_mutex_lock(mutex) @@ -48,6 +49,11 @@ int dummy_pthread_join(pthread_t pthread, void **retval); int dummy_pthread_init(void *); +#define PTHREAD_ONCE_INIT 0 +int nothreads_pthread_once(pthread_once_t *once_control, + void (*init_routine)(void)); +#define pthread_once(once, routine) nothreads_pthread_once(once, routine) + #endif int online_cpus(void);
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> --- 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? compat/win32/pthread.c | 22 ++++++++++++++++++++++ compat/win32/pthread.h | 5 +++++ thread-utils.c | 11 +++++++++++ thread-utils.h | 6 ++++++ 4 files changed, 44 insertions(+)