Hi,
the comment is wrong.
'ap_sub_req_lookup_dirent' uses the fact that 'rnew->uri' has some extra
space after the NUL.
'rnew->uri' is allocated via 'ap_escape_uri' which is defined as:
#define ap_escape_uri(ppool,path) ap_os_escape_path(ppool,path,1)
So, what matters here is the case in 'ap_os_escape_path' where partial
*is* set.
However, I have commited r1589599 in order to update the comment and to
allocate one extra byte to be safe, even if not needed in this
particular case.
Best regards,
CJ
Le 22/04/2014 11:03, Yann Ylavic a écrit :
On Tue, Apr 22, 2014 at 9:47 AM, Christophe JAILLET
<[email protected]> wrote:
The first part of the comment, against 'ap_os_escape_path', is, IMO, wrong.
We are not guaranteed that, if partial is *not* set, that there will be one
byte of additional space after the NUL.
If partial *is* set, then we skip the :
*d++ = '.';
*d++ = '/';
and extra bytes will be available after the NUL.
But if it is *not* set, I think that there may be (unlikely) cases where not
space is available at the end.
So, either the comment should be updated or 1 extra byte should be
allocated, to be safe. In this later case, the comment should also be
updated to state that in *all* cases, one extra byte is available.
+1 for allocating an extra byte.
+1 for extra byte too, appending '/' without (re)allocating is quite useful.
Moreover, 'ap_os_escape_path' could be tweaked as in r1485723.
-0, it's a compromise between speed and space, filenames (paths) are
often shorter than logs (and maybe less subject to escaping), I'm not
sure it's worth the second loop in this case.
Regards,
Yann.
