You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2016/05/26 13:16:45 UTC

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Excellent, but one big issue, namespace collision.

What about...

+#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)
++/**+ * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
+AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);


On Thu, May 26, 2016 at 3:52 AM, <ja...@apache.org> wrote:

> Author: jailletc36
> Date: Thu May 26 08:52:09 2016
> New Revision: 1745582
>
> URL: http://svn.apache.org/viewvc?rev=1745582&view=rev
> Log:
> Proposal
>
> Modified:
>     httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582&r1=1745581&r2=1745582&view=diff
>
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>       2.4.x patch: trunk works
>       +1: jailletc36,
>
> +  *) core: ASCII string comparison functions optimized speed.
> +     This proposal includes and renames ap_casecmpstr[n] functions
> available
> +     in trunk.
> +     The proposed names are the ones used in APR for the same kind of
> functions.
> +     In order to avoid some new ap_ functions (which are just copies of
> what is
> +     available in APR 1.6.0+), I propose to use exactly the same names
> and only
> +     declare and define the functions in httpd if not available in APR.
> +     The same approach has already been used for apr_time_from_msec() for
> example.
> +     Also note that the implementation in APR and in httpd are slighly
> different.
> +     If/when aggreed on this backport and function names in httpd, then
> trunk should
> +     be upgraded accordingly. Uses of the functions could then be
> backported.
> +     trunk patch: ?
> +     2.4.x patch:
> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
> +     +1: jailletc36,
> +
>  PATCHES/ISSUES THAT ARE BEING WORKED
>
>    *) http: Don't remove the Content-Length of zero from a HEAD response if
>
>
>

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
>
> On Thu, May 26, 2016 at 3:52 AM, <ja...@apache.org> wrote:
>>
>>> Author: jailletc36
>>> Date: Thu May 26 08:52:09 2016
>>> New Revision: 1745582
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1745582&view=rev
>>> Log:
>>> Proposal
>>>
>>> Modified:
>>>     httpd/httpd/branches/2.4.x/STATUS
>>>
>>> Modified: httpd/httpd/branches/2.4.x/STATUS
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582&r1=1745581&r2=1745582&view=diff
>>>
>>> ==============================================================================
>>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>>       2.4.x patch: trunk works
>>>       +1: jailletc36,
>>>
>>> +  *) core: ASCII string comparison functions optimized speed.
>>> +     This proposal includes and renames ap_casecmpstr[n] functions
>>> available
>>> +     in trunk.
>>> +     The proposed names are the ones used in APR for the same kind of
>>> functions.
>>> +     In order to avoid some new ap_ functions (which are just copies of
>>> what is
>>> +     available in APR 1.6.0+), I propose to use exactly the same names
>>> and only
>>> +     declare and define the functions in httpd if not available in APR.
>>> +     The same approach has already been used for apr_time_from_msec()
>>> for example.
>>> +     Also note that the implementation in APR and in httpd are slighly
>>> different.
>>> +     If/when aggreed on this backport and function names in httpd, then
>>> trunk should
>>> +     be upgraded accordingly. Uses of the functions could then be
>>> backported.
>>> +     trunk patch: ?
>>> +     2.4.x patch:
>>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>>> +     +1: jailletc36,
>>> +
>>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>>
>>>    *) http: Don't remove the Content-Length of zero from a HEAD response
>>> if
>>>
>>>
>>>
>>
>

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Premature ctrl+enter... sorry about that...

+#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)
> ++/** * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
> +AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);
>
> On Thu, May 26, 2016 at 3:52 AM, <ja...@apache.org> wrote:
>
>> Author: jailletc36
>> Date: Thu May 26 08:52:09 2016
>> New Revision: 1745582
>>
>> URL: http://svn.apache.org/viewvc?rev=1745582&view=rev
>> Log:
>> Proposal
>>
>> Modified:
>>     httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582&r1=1745581&r2=1745582&view=diff
>>
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>       2.4.x patch: trunk works
>>       +1: jailletc36,
>>
>> +  *) core: ASCII string comparison functions optimized speed.
>> +     This proposal includes and renames ap_casecmpstr[n] functions
>> available
>> +     in trunk.
>> +     The proposed names are the ones used in APR for the same kind of
>> functions.
>> +     In order to avoid some new ap_ functions (which are just copies of
>> what is
>> +     available in APR 1.6.0+), I propose to use exactly the same names
>> and only
>> +     declare and define the functions in httpd if not available in APR.
>> +     The same approach has already been used for apr_time_from_msec()
>> for example.
>> +     Also note that the implementation in APR and in httpd are slighly
>> different.
>> +     If/when aggreed on this backport and function names in httpd, then
>> trunk should
>> +     be upgraded accordingly. Uses of the functions could then be
>> backported.
>> +     trunk patch: ?
>> +     2.4.x patch:
>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>> +     +1: jailletc36,
>> +
>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>
>>    *) http: Don't remove the Content-Length of zero from a HEAD response
>> if
>>
>>
>>
>

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Posted by Yann Ylavic <yl...@gmail.com>.
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.

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
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
+
+/**
+ * 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.

Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
This will teach me to go back to pine... gui web clients... grrr...

Final draft...

On Thu, May 26, 2016 at 8:16 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> Excellent, but one big issue, namespace collision.
>
> What about...
>

+#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)
+
+/**
+ * HTTPD Internal function, do not use. @see apr_cstr_casecmp instead
+ */
+AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);

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?

On Thu, May 26, 2016 at 3:52 AM, <ja...@apache.org> wrote:
>
>> Author: jailletc36
>> Date: Thu May 26 08:52:09 2016
>> New Revision: 1745582
>>
>> URL: http://svn.apache.org/viewvc?rev=1745582&view=rev
>> Log:
>> Proposal
>>
>> Modified:
>>     httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582&r1=1745581&r2=1745582&view=diff
>>
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>       2.4.x patch: trunk works
>>       +1: jailletc36,
>>
>> +  *) core: ASCII string comparison functions optimized speed.
>> +     This proposal includes and renames ap_casecmpstr[n] functions
>> available
>> +     in trunk.
>> +     The proposed names are the ones used in APR for the same kind of
>> functions.
>> +     In order to avoid some new ap_ functions (which are just copies of
>> what is
>> +     available in APR 1.6.0+), I propose to use exactly the same names
>> and only
>> +     declare and define the functions in httpd if not available in APR.
>> +     The same approach has already been used for apr_time_from_msec()
>> for example.
>> +     Also note that the implementation in APR and in httpd are slighly
>> different.
>> +     If/when aggreed on this backport and function names in httpd, then
>> trunk should
>> +     be upgraded accordingly. Uses of the functions could then be
>> backported.
>> +     trunk patch: ?
>> +     2.4.x patch:
>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>> +     +1: jailletc36,
>> +
>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>
>>    *) http: Don't remove the Content-Length of zero from a HEAD response
>> if
>>
>>
>>
>