[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: HAVE_GMTIME_R?



> That would mean moving lutil_gettime() (and its caller lutil_csnstr()?)
> to libldap, so libldap_r can use the mutex.

Well, I was about to discuss this.  In fact, there's a few comments in the
code that seem to indicate the gmtime_mutex is used for other purposes
than calling gmtime(3) safely.  I mean: I plan to use gmtime(3) this way,

#ifdef USE_GMTIME_R
#define ldap_pvt_gmtime(t,tm) gmtime_r((t), (tm))
#else
struct tm *ldap_pvt_gmtime(time_t *t, struct tm *tm)
{
    struct tm *tm_ptr;
#ifdef LDAP_R_COMPILE
    ldap_pvt_thread_mutex_lock( &gmtime_mutex );
#endif
    tm_ptr = gmtime( t );
    *tm = *tm_ptr;
#ifdef LDAP_R_COMPILE
    ldap_pvt_thread_mutex_unlock( &gmtime_mutex );
#endif
    return tm;
}

This means that the result of calling ldap_pvt_gmtime() will always be
safe.  However, somewhere in the code there was things like

    ldap_pvt_thread_mutex_lock( &gmtime_mutex );
    // do something that implies calling gmtime()
    ldap_pvt_thread_mutex_unlock( &gmtime_mutex );

I guess the only reason was to avoid copying the contents of the structure
returned by gmtime(3).  I need to check whether my assumption is correct
or not.  Significant cases include calling lutil_csnstr().

> But not exposing gmtime_mutex would break third-partly modules that
> use gmtime()/localtime() correctly, which means protecting them with
> gmtime_mutex.  We could however register gmtime_mutex in libldap,
> similar to registering which memory functions to use.

Right.  But as far as I can tell, this is already the case of other
mutex'es, including ldap_int_ctime_mutex, which is static, and other
ldap_int_*_mutex'es, that are internal to libldap.

I'd rather favor exposing an API that allows to lock/unlock (trylock?)
hidden mutexes.  Although I fear this could open a can of worms.

p.