On Thu, May 26, 2016 at 3:37 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Thu, May 26, 2016 at 8:24 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>>
>>
>>> Excellent, but one big issue, namespace collision.
>>>
>>> [...]
>>
>> would be the proper doxygen, to dissuade users from directly consuming
>> ap_ flavor.  However we would not drop the ap_flavor until 2.6.x so that
>> any
>> later updates of 2.4.x would still retain this function.
>>
>> When the user updates to apr 1.6.x without altering httpd 2.4.21, we don't
>> want the symbol collisions between the httpd binary and libapr-1.so
>> library.
>> The effects vary between OS architectures, from inconvenient to fatal to
>> entirely benign (win32 or os2 2-level namespaces).
>>
>> WDYT?
>
>
> I overlooked something, we can simplify... in the header...
>
> +#if !APR_VERSION_AT_LEAST(1,6,0)
> +/**
> + * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
> + * @param s1 The 1st string to compare
> + * @param s2 The 2nd string to compare
> + * @return 0 if s1 is lexicographically equal to s2 ignoring case;
> + *         non-0 otherwise.
> + */
> +#define apr_cstr_casecmp(s1, s2) ap_cstr_casecmp(s1, s2)
> +#endif

Why would we provide an APR thing in httpd ?

> +
> +/**
> + * HTTPD Internal function, do not use. @see apr_cstr_casecmp instead
> + * @deprecated Internal function will be absent from httpd 2.6 / 3.0.
> + */
> +AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);
>
> Note the function declaration is a constant, above, not conditional.
>
> In the C sources...
>
> +#if APR_VERSION_AT_LEAST(1,6,0)
> +int ap_cstr_casecmp(const char *s1, const char *s2)
> +{
> +    return apr_cstr_casecmp(const char *s1, const char *s2);
> +}
> +#else
> +int ap_cstr_casecmp(const char *s1, const char *s2)
> +{
> ... full implementation.
>
> This ensures any earlier module code linked to ap_cstr... will still
> resolve that symbol, while transitioning to a call out to apr_cstr
> flavor once httpd is recompiled with the newer apr.
>
> This seems like the best transitional approach.

ISTM that modules could use ap_cstr_casecmp() w/o ever transitioning,
what's the downside since we will never deprecate it, or will we?

BTW, why not simply:
#if APR_VERSION_AT_LEAST(1,6,0)
#define ap_cstr_casecmp apr_cstr_casecmp
#else
AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);
#endif
so that we don't provide something in the APR namespace?

Regards,
Yann.

Reply via email to