On 3/24/25 6:24 PM, Graham Leggett wrote:
> On 20 Mar 2025, at 16:56, Ruediger Pluem <rpl...@apache.org> wrote:
>
>>> This code is compiled with openldap and ldap_connect does not seem to be
>>> present with openldap at least not with 2.4.46 which is
>>> delivered with RedHat 8. I can see it though in openldap 2.6.6 which is
>>> delivered with RedHat 9
>>>
>>> ldap/apr_ldap.c: In function ‘apr__ldap_connect’:
>>> ldap/apr_ldap.c:1091:15: warning: implicit declaration of function
>>> ‘ldap_connect’; did you mean ‘ldap_cancel’?
>>> [-Wimplicit-function-declaration]
>>> err->rc = ldap_connect(ld);
>>> ^~~~~~~~~~~~
>>> ldap_cancel
>>
>> Thanks to r1924295 this compiles now, but it creates just a stub for
>> apr_ldap_connect that always returns APR_ENOTIMPL.
>> As apr_ldap_connect is a central function of the new API I think it would be
>> better to fail during configure time
>> if ldap_connect is not found instead of creating a binary with an unusable
>> API. Maybe a check for an appropriate version
>> of openldap (>= 2.5 looks like to be fine) in case openldap is the library
>> being used would be also helpful to give the
>> user compiling the code a better hint what is required to make the API work.
>
> I encountered a missing ldap_connect() on the ancient openldap that ships
> with latest MacOS. While ldap_connect() is present on
> Windows and reasonably modern openldap, ldap_connect() is missing on IBM
> Tivoli, meaning there would be no way to ever support
> these platforms in future.
>
> ldap_connect() is marked as optional in the APIs that support it, the point
> is have a clear location where you can catch and
> return connect errors, instead of encountering them in a search or a modify.
> If the function is not there, it is not the end of
> the world.
>
> The test case handles a possibly missing ldap_connect like this:
>
> https://github.com/apache/apr/blob/trunk/test/testldap.c#L991
>
> I think the best approach is to let the caller decide if they care or not. If
> you care, you might get APR_ENOTIMPL and can respond
> accordingly (fail, work around). If you don't care, you don't use
> ldap_connect() at all. The API presents the underlying reality
> to you, and you get to decide what to do.
But shouldn't an API in APR abstract these details away from the user of the
API? When I want to use this API on MacOS or an older
Linux distribution I either cannot or I need to add extra code (given the
example in testldap.c quite some lines) which is likely
always the same in every project where I want to use the API. While it gives
people more flexibility to have apr_ldap_connect as
it currently is as it allows them to define the workaround for the missing
ldap_connect on their own shouldn't there be a default
implementation of a workaround available ready to call when apr_ldap_connect
returns APR_ENOTIMPL?
Regards
Rüdiger
