Re: svn commit: r1917094 [2/3] - in /apr/apr/trunk: ./ build/ docs/ include/ include/private/ ldap/ misc/unix/ test/

On 3/24/25 6:24 PM, Graham Leggett wrote:
> On 20 Mar 2025, at 16:56, Ruediger Pluem <[email protected]> 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