> Looks good in general, see comments below > > On 2014-10-28 20:46, Peter Meerwald wrote: > > From: Peter Meerwald <p.meerwald at bct-electronic.com> > > Missing a good commit message. > > > Signed-off-by: Peter Meerwald <pmeerw at pmeerw.net> > > --- > > src/pulsecore/once.c | 18 +----------------- > > src/pulsecore/once.h | 27 +++++++++++++++++++++++---- > > 2 files changed, 24 insertions(+), 21 deletions(-) > > > > diff --git a/src/pulsecore/once.c b/src/pulsecore/once.c > > index 16059c3..cac8cda 100644 > > --- a/src/pulsecore/once.c > > +++ b/src/pulsecore/once.c > > @@ -30,14 +30,9 @@ > > /* See http://www.hpl.hp.com/research/linux/atomic_ops/example.php4 for > the > > * reference algorithm used here. */ > > > > -bool pa_once_begin(pa_once *control) { > > +bool pa_once_doit(pa_once *control) { > > pa_mutex *m; > > > > - pa_assert(control); > > - > > - if (pa_atomic_load(&control->done)) > > - return false; > > - > > /* Caveat: We have to make sure that the once func has completed > > * before returning, even if the once func is not actually > > * executed by us. Hence the awkward locking. */ > > @@ -64,14 +59,3 @@ void pa_once_end(pa_once *control) { > > m = pa_static_mutex_get(&control->mutex, false, false); > > pa_mutex_unlock(m); > > } > > - > > -/* Not reentrant -- how could it be? */ > > -void pa_run_once(pa_once *control, pa_once_func_t func) { > > - pa_assert(control); > > - pa_assert(func); > > - > > - if (pa_once_begin(control)) { > > - func(); > > - pa_once_end(control); > > - } > > -} > > diff --git a/src/pulsecore/once.h b/src/pulsecore/once.h > > index 460a700..3d528a7 100644 > > --- a/src/pulsecore/once.h > > +++ b/src/pulsecore/once.h > > @@ -26,18 +26,27 @@ > > #include <pulsecore/mutex.h> > > > > typedef struct pa_once { > > - pa_static_mutex mutex; > > pa_atomic_t done; > > + pa_static_mutex mutex; > > Any particular reason these changed order? ah, I was experimenting with alignment of atomic variables but I have no good argument either way -- should keep the original order > > > } pa_once; > > > > #define PA_ONCE_INIT \ > > { \ > > + .done = PA_ATOMIC_INIT(0), \ > > .mutex = PA_STATIC_MUTEX_INIT, \ > > - .done = PA_ATOMIC_INIT(0) \ > > Ditto > > > } > > > > /* Not to be called directly, use the macros defined below instead */ > > -bool pa_once_begin(pa_once *o); > > +bool pa_once_doit(pa_once *control); > > You would typically not like to expose this function, but it is > necessary as the beginning of the function was inlined. Should perhaps > be called "pa_once_begin_internal" and have a comment saying that one > should call pa_once_begin instead of this function. will do, thanks! > > > + > > +static inline bool pa_once_begin(pa_once *control) { > > + pa_assert(control); > > + > > + if (pa_atomic_load(&control->done)) > > + return false; > > + > > + return pa_once_doit(control); > > +} > > void pa_once_end(pa_once *o); > > > > #define PA_ONCE_BEGIN \ > > @@ -68,6 +77,16 @@ void pa_once_end(pa_once *o); > > > > /* Same API but calls a function */ > > typedef void (*pa_once_func_t) (void); > > -void pa_run_once(pa_once *o, pa_once_func_t f); > > + > > +/* Not reentrant -- how could it be? */ > > +static inline void pa_run_once(pa_once *control, pa_once_func_t func) { > > + pa_assert(control); > > + pa_assert(func); > > + > > + if (pa_once_begin(control)) { > > + func(); > > + pa_once_end(control); > > + } > > +} > > > > #endif > > > > -- Peter Meerwald +43-664-2444418 (mobile)
