On Tue, 8 Nov 2011, "Plüm, Rüdiger, VF-Group" wrote:
-----Original Message-----
From: Stefan Fritsch [mailto:[email protected]]
Sent: Dienstag, 8. November 2011 03:15
To: [email protected]
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.

Reply via email to