On 03/07/2016 12:53 PM, Yann Ylavic wrote:
> [resend: list's reply policy strikes again :/ ]
>
> On Mon, Mar 7, 2016 at 12:47 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>> On Mon, Mar 7, 2016 at 10:52 AM, Ruediger Pluem <rpl...@apache.org> wrote:
>>>
>>> On 03/06/2016 01:19 AM, yla...@apache.org wrote:
>>>>
>>>> /* Implement OS-specific accessors defined in apr_portable.h */
>>>>
>>>> -apr_status_t apr_os_proc_mutex_get(apr_os_proc_mutex_t *ospmutex,
>>>> - apr_proc_mutex_t *pmutex)
>>>> +APR_DECLARE(apr_status_t) apr_os_proc_mutex_get_ex(apr_os_proc_mutex_t
>>>> *ospmutex,
>>>> + apr_proc_mutex_t
>>>> *pmutex,
>>>> + apr_lockmech_e *mech)
>>>> +{
>>>> +#if 1
>>>> + /* We need to change apr_os_proc_mutex_t to a pointer type
>>>> + * to be able to implement this function.
>>>> + */
>>>> + return APR_ENOTIMPL;
>>>> +#else
>>>> + if (!pmutex->mutex) {
>>>> + return APR_ENOLOCK;
>>>> + }
>>>> + *ospmutex = pmutex->mutex->mutex;
>>>> + if (mech) {
>>>> + *mech = APR_LOCK_DEFAULT;
>>>> + }
>>>> + return APR_SUCCESS;
>>>> +#endif
>>>> +}
>>>> +
>>>> +APR_DECLARE(apr_status_t) apr_os_proc_mutex_get(apr_os_proc_mutex_t
>>>> *ospmutex,
>>>> + apr_proc_mutex_t *pmutex)
>>>> {
>>>> - if (pmutex)
>>>> - ospmutex = pmutex->mutex->mutex;
>>>> - return APR_ENOLOCK;
>>>> + return apr_os_proc_mutex_get_ex(ospmutex, pmutex, NULL);
>>>> }
>>>
>>> Previously we always returned APR_ENOLOCK now we return always APR_ENOTIMPL
>>> and ospmutex will not be set.
>>> Is this correct?
>>
>> The pointer ospmutex is local here, there is no point in changing its
>> value, that won't help the caller :)
>> The issue is that on Netware, apr_os_proc_mutex_t is the native
>> NXMutex_t struct, so we can't really return a copy to the caller with
>> something like "*ospmutex = *pmutex->mutex;" either since it is likely
>> to not work as expected...
Thanks for explaining.
>> That's why I changed to apr_os_proc_mutex_t to NXMutex_t* in the
>> follow up (r1733777), which breaks the API, whereas this commit aims
>> to be backportable to 1.6 (still apr_os_proc_mutex_get() is
>> ENOTIMPLementable...).
>>
>> However I should have leaved (unconditionally) what is done in
>> r1733777 (and in other platforms) wrt ENOLOCK:
>>
>> if (!pmutex->mutex) {
>> return APR_ENOLOCK;
>> }
>> #if 1
>> /* ... */
>> return APR_ENOTIMPL;
>> #else
>> ...
>> #endif
>>
>> Does it work for you if I take care of this when/if backporting (since
>> trunk is already fixed)?
This is fine for me.
Regards
Rüdiger
