svn commit: r1898286 - /httpd/httpd/trunk/server/util_expr_eval.c

[ja...@apache.org]

Author: jailletc36
Date: Mon Feb 21 21:07:35 2022
New Revision: 1898286

URL: http://svn.apache.org/viewvc?rev=1898286&view=rev
Log:
There is no point in calling ap_varbuf_grow() here, it is already
called from within ap_varbuf_strmemcat().

Moreover, 2nd parameter should be the minimum total new length, not
the amount of the growth. So this call is likely to be a no-op.

Modified:
    httpd/httpd/trunk/server/util_expr_eval.c

Modified: httpd/httpd/trunk/server/util_expr_eval.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_expr_eval.c?rev=1898286&r1=1898285&r2=1898286&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_expr_eval.c (original)
+++ httpd/httpd/trunk/server/util_expr_eval.c Mon Feb 21 21:07:35 2022
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-/*                      _             _
+/*
  *  ap_expr_eval.c, based on ssl_expr_eval.c from mod_ssl
  */
 
@@ -124,7 +124,6 @@ static const char *ap_expr_list_pstrcat(
         for (i = 0; i < n; ++i) {
             val = APR_ARRAY_IDX(list, i, const char*);
             vlen = strlen(val);
-            ap_varbuf_grow(&vb, vlen + slen + 1);
             ap_varbuf_strmemcat(&vb, val, vlen);
             ap_varbuf_strmemcat(&vb, sep, slen);
         }

Re: svn commit: r1898286 - /httpd/httpd/trunk/server/util_expr_eval.c

[Ruediger Pluem <rp...@apache.org>]

On 2/21/22 10:07 PM, jailletc36@apache.org wrote:
> Author: jailletc36
> Date: Mon Feb 21 21:07:35 2022
> New Revision: 1898286
> 
> URL: http://svn.apache.org/viewvc?rev=1898286&view=rev
> Log:
> There is no point in calling ap_varbuf_grow() here, it is already
> called from within ap_varbuf_strmemcat().
> 
> Moreover, 2nd parameter should be the minimum total new length, not
> the amount of the growth. So this call is likely to be a no-op.

I agree that the current usage of ap_varbuf_grow is wrong as it should be the complete length and not the growth.
OTOH the idea seems to be to avoid too much reallocations and memcpys here.
We could precalculate the length, but I am not sure if this creates more overhead than needed here. If we precalculate the length
the question is also if the ap_varbuf_ is still the best api for this or we should switch to apr_pstrcatv instead to avoid
having to strlen all elements of the list twice. Of course this costs a little more memory for the struct iovec array.

Regards

Rüdiger