httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject RE: Fwd: svn commit: r1197405 - in /httpd/httpd/trunk: CHANGES docs/manual/upgrading.xml modules/filters/mod_substitute.c
Date Tue, 08 Nov 2011 19:43:31 GMT
On Tue, 8 Nov 2011, "Plüm, Rüdiger, VF-Group" wrote:
>> -----Original Message-----
>> From: Stefan Fritsch [mailto:sf@sfritsch.de]
>> Sent: Dienstag, 8. November 2011 03:15
>> To: dev@httpd.apache.org
>> Subject: Re: Fwd: svn commit: r1197405 - in
>> /httpd/httpd/trunk: CHANGES docs/manual/upgrading.xml
>> modules/filters/mod_substitute.c
>>
>> On Sun, 6 Nov 2011, Stefan Fritsch wrote:
>>>>>> Isn't it sufficient to reduce space_left only by repl_len?
>>>>>> IMHO we only allocate repl_len new memory in
>> ap_pregsub_ex and not len
>>>>>> + repl_len or am I wrong here?
>>>>>>
>>>>>>
>>>>
>>>>
>>>> Any comment on that?
>>>
>>> That was also supposed to cause the line length to be
>> limited instead of the
>>> length of the replacement. But you are right, this is not
>> correct. I will
>>> look at it during the hackathon.
>>
>> I have looked more closely, and the code is actually correct.
>> It's purpose
>> is not to limit new memory allocation but the line length. I
>> have added a
>> few comments in r1199060 which hopefully make the intentions clear.
>>
>
>                             len = (apr_size_t) (regm[0].rm_eo - regm[0].rm_so);
> +                            repl_len = strlen(repl);
> +                            space_left -= len + repl_len;
>
>
> But len is the length of the string we remove correct?

Oops. True, I looked at the wrong part of the code where len has a 
different meaning :-(

> So the limit will be imposed on the sum of the length of the strings to replace plus
the
> sum of all replacements for this line. So actually the length of the original
> and the resulting line can be much shorter than the allowed limit. Example:
>
> Lets say the limit is 10 chars per line.
> The source line is: 12345
> I do a s/12345/123456/.
> This would fail because the length of the string to replace was 5 and that of the replacement
was 6.
> But the resulting line only would have a length of 6.
>
> But s/12345/123456/n would work (so not using regex).
>
> Does this approach really make sense and is comprehensible for users?
> Or do I simply fail to correctly understand the code?
>
> Maybe space_left above is meant to be
>
> space_left -= repl_len + (apr_size_t) (regm[0].rm_so - pos);

regm[0].rm_so is already counted from pos, but that's basically it. I hope 
I got it right in r1199410. Thanks again.
Mime
  • Unnamed multipart/mixed (inline, None, 0 bytes)
View raw message