You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2014/08/11 02:04:32 UTC
Re: svn commit: r1617148 - /subversion/branches/authzperf/subversion/libsvn_repos/authz.c
On 10.08.2014 21:17, stefan2@apache.org wrote:
> Author: stefan2
> Date: Sun Aug 10 19:17:53 2014
> New Revision: 1617148
[...]
> @@ -973,7 +869,11 @@ add_complex_matches(lookup_state_t *stat
> for (i = 0; i < patterns->nelts; ++i)
> {
> node_t *node = APR_ARRAY_IDX(patterns, i, node_t *);
> - if (match_pattern(segment->data, node->segment.data))
> +
> + /* There should be not slashes and periods should be treated as
> + * literals. */
> + if (APR_SUCCESS == apr_fnmatch(node->segment.data, segment->data,
> + APR_FNM_PATHNAME | APR_FNM_PERIOD))
> add_next_node(state, node);
I assume there's no way to get slashes in a segment in any case, right?
So APR_FNM_PATHNAME is redundant. I wonder however why you think that we
need APR_FNM_PERIOD?
Oh, and BTW: please don't compare to APR_SUCCESS. We don't use that to
check the results of strcmp, either.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com
Re: svn commit: r1617148 - /subversion/branches/authzperf/subversion/libsvn_repos/authz.c
Posted by Branko Čibej <br...@wandisco.com>.
On 13 Aug 2014 10:10, "Stefan Fuhrmann" <st...@wandisco.com>
wrote:
>
> On Mon, Aug 11, 2014 at 2:04 AM, Branko Čibej <br...@wandisco.com> wrote:
>>
>> On 10.08.2014 21:17, stefan2@apache.org wrote:
>>>
>>> Author: stefan2
>>> Date: Sun Aug 10 19:17:53 2014
>>> New Revision: 1617148
>>
>> [...]
>>>
>>> @@ -973,7 +869,11 @@ add_complex_matches(lookup_state_t *stat
>>> for (i = 0; i < patterns->nelts; ++i)
>>> {
>>> node_t *node = APR_ARRAY_IDX(patterns, i, node_t *);
>>> - if (match_pattern(segment->data, node->segment.data))
>>> +
>>> + /* There should be not slashes and periods should be treated as
>>> + * literals. */
>>> + if (APR_SUCCESS == apr_fnmatch(node->segment.data,
segment->data,
>>> + APR_FNM_PATHNAME |
APR_FNM_PERIOD))
>>> add_next_node(state, node);
>>
>>
>>
>> I assume there's no way to get slashes in a segment in any case, right?
So APR_FNM_PATHNAME is redundant. I wonder however why you think that we
need APR_FNM_PERIOD?
>
>
> I was under the impression that a '.' in the pattern might either be
> ignored (as per commentary in apr_fnmatch.c) or be wildcard match
> similar to '?' The header docs are not really helpful there. In any case,
> I wanted periods to only match periods.
>
> Reading the source code now, I found that this flag is about limiting
> the reach of '*'. With APR_FNM_PERIOD set, it will not match beyond
> a period. That's not what I want.
>
>>
>>
>> Oh, and BTW: please don't compare to APR_SUCCESS. We don't use that to
check the results of strcmp, either.
>
>
> All changed in r1617693.
Just a note about apr_fnmatch and its return code: regardless of the
prototype, this function does *not* return an APR status code. Hence, using
APR_SUCCESS is semantically wrong ...
Re: svn commit: r1617148 - /subversion/branches/authzperf/subversion/libsvn_repos/authz.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Aug 11, 2014 at 2:04 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 10.08.2014 21:17, stefan2@apache.org wrote:
>
> Author: stefan2
> Date: Sun Aug 10 19:17:53 2014
> New Revision: 1617148
>
> [...]
>
> @@ -973,7 +869,11 @@ add_complex_matches(lookup_state_t *stat
> for (i = 0; i < patterns->nelts; ++i)
> {
> node_t *node = APR_ARRAY_IDX(patterns, i, node_t *);
> - if (match_pattern(segment->data, node->segment.data))
> +
> + /* There should be not slashes and periods should be treated as
> + * literals. */
> + if (APR_SUCCESS == apr_fnmatch(node->segment.data, segment->data,
> + APR_FNM_PATHNAME | APR_FNM_PERIOD))
> add_next_node(state, node);
>
>
>
> I assume there's no way to get slashes in a segment in any case, right? So
> APR_FNM_PATHNAME is redundant. I wonder however why you think that we need
> APR_FNM_PERIOD?
>
I was under the impression that a '.' in the pattern might either be
ignored (as per commentary in apr_fnmatch.c) or be wildcard match
similar to '?' The header docs are not really helpful there. In any case,
I wanted periods to only match periods.
Reading the source code now, I found that this flag is about limiting
the reach of '*'. With APR_FNM_PERIOD set, it will not match beyond
a period. That's not what I want.
>
> Oh, and BTW: please don't compare to APR_SUCCESS. We don't use that to
> check the results of strcmp, either.
>
All changed in r1617693.
-- Stefan^2.