You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arwin Arni <ar...@collab.net> on 2011/01/31 10:48:17 UTC
[PATCH in progress] Ref. Issue#3781 Make parsing section names case
sensitive
Hi All,
This patch revs subversion/libsvn_subr/config.c:svn_config_read to
svn_config_read2 to accept a *boolean* called
*section_names_case_sensitive*.
When TRUE, the section names are populated case sensitively in the config.
This was done mainly for the case insensitivity issue in authz file
parsing.
(Issue #3781 <http://subversion.tigris.org/issues/show_bug.cgi?id=3781>
-> http://subversion.tigris.org/issues/show_bug.cgi?id=3781 )
Here is a relevant thread: http://svn.haxx.se/dev/archive-2011-01/0441.shtml
Currently, this patch will only treat section names case sensitively.
Should I add parameters for options / values / options & values to
decide the case or shall I use a common parameter for all(sections,
options & values) ?
Kindly share your thoughts on this.
Regards,
Arwin Arni
Re: [PATCH in progress] Ref. Issue#3781 Make parsing section names
case sensitive
Posted by Arwin Arni <ar...@collab.net>.
On Monday 31 January 2011 08:49 PM, C. Michael Pilato wrote:
> On 01/31/2011 10:05 AM, Arwin Arni wrote:
>> On Monday 31 January 2011 08:01 PM, C. Michael Pilato wrote:
>>> On 01/31/2011 04:48 AM, Arwin Arni wrote:
>>>> Should I add parameters for options / values / options& values to decide
>>>> the case or shall I use a common parameter for all(sections, options&
>>>> values) ?
>>> We generally try to avoid premature over-engineering, and at the moment, we
>>> have no known need for the additional changes you speak of. Introducing
>>> codepaths that we don't exercise can lead to surprise failures later when we
>>> do.
>> But I am excercising it!
>>
>> In libsvn_repos/authz.c. I'm using it to make the config population
>> case-sensitive. (Issue #3781)
> Unless I'm mistaken, your immediate need is to make *section* parsing
> case-sensitive. Nothing about issue #3781 calls for toggle-able case
> handling of options or values. *That's* the "additional changes" stuff I'm
> referring to.
>
Oh! Okay.. It was originally Kamesh's idea to do this.. so.. I guess
I'll have to get back to you after a few more words with him..
Thanks for the feedback..
Regards,
Arwin Arni
Re: [PATCH in progress] Ref. Issue#3781 Make parsing section names
case sensitive
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/31/2011 10:05 AM, Arwin Arni wrote:
> On Monday 31 January 2011 08:01 PM, C. Michael Pilato wrote:
>> On 01/31/2011 04:48 AM, Arwin Arni wrote:
>>> Should I add parameters for options / values / options& values to decide
>>> the case or shall I use a common parameter for all(sections, options&
>>> values) ?
>>
>> We generally try to avoid premature over-engineering, and at the moment, we
>> have no known need for the additional changes you speak of. Introducing
>> codepaths that we don't exercise can lead to surprise failures later when we
>> do.
> But I am excercising it!
>
> In libsvn_repos/authz.c. I'm using it to make the config population
> case-sensitive. (Issue #3781)
Unless I'm mistaken, your immediate need is to make *section* parsing
case-sensitive. Nothing about issue #3781 calls for toggle-able case
handling of options or values. *That's* the "additional changes" stuff I'm
referring to.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH in progress] Ref. Issue#3781 Make parsing section names
case sensitive
Posted by Arwin Arni <ar...@collab.net>.
On Monday 31 January 2011 08:01 PM, C. Michael Pilato wrote:
> On 01/31/2011 04:48 AM, Arwin Arni wrote:
>> Hi All,
>>
>> This patch revs subversion/libsvn_subr/config.c:svn_config_read to
>> svn_config_read2 to accept a *boolean* called *section_names_case_sensitive*.
>>
>> When TRUE, the section names are populated case sensitively in the config.
>>
>> This was done mainly for the case insensitivity issue in authz file parsing.
>> (Issue #3781<http://subversion.tigris.org/issues/show_bug.cgi?id=3781> ->
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3781 )
>>
>> Here is a relevant thread: http://svn.haxx.se/dev/archive-2011-01/0441.shtml
>>
>> Currently, this patch will only treat section names case sensitively.
>>
>> Should I add parameters for options / values / options& values to decide
>> the case or shall I use a common parameter for all(sections, options& values) ?
>
> We generally try to avoid premature over-engineering, and at the moment, we
> have no known need for the additional changes you speak of. Introducing
> codepaths that we don't exercise can lead to surprise failures later when we
> do.
But I am excercising it!
In libsvn_repos/authz.c. I'm using it to make the config population
case-sensitive. (Issue #3781)
All other callers pass FALSE to that parameter (unless they suddenly
require case-sensitivity).
I hope I've made things a little clearer. :)
> That said, I suppose if you're able to give sufficient unit test
> coverage, that's enough "exercise". I could go either way on this one.
> (Sorry... I don't suppose that was very helpful, was it?)
>
A colleague of mine has already started working on a test case for this.
Looking forward to hear more about this..
Thanks and regards,
Arwin Arni
Re: [PATCH in progress] Ref. Issue#3781 Make parsing section names
case sensitive
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/31/2011 04:48 AM, Arwin Arni wrote:
> Hi All,
>
> This patch revs subversion/libsvn_subr/config.c:svn_config_read to
> svn_config_read2 to accept a *boolean* called *section_names_case_sensitive*.
>
> When TRUE, the section names are populated case sensitively in the config.
>
> This was done mainly for the case insensitivity issue in authz file parsing.
> (Issue #3781 <http://subversion.tigris.org/issues/show_bug.cgi?id=3781> ->
> http://subversion.tigris.org/issues/show_bug.cgi?id=3781 )
>
> Here is a relevant thread: http://svn.haxx.se/dev/archive-2011-01/0441.shtml
>
> Currently, this patch will only treat section names case sensitively.
>
> Should I add parameters for options / values / options & values to decide
> the case or shall I use a common parameter for all(sections, options & values) ?
We generally try to avoid premature over-engineering, and at the moment, we
have no known need for the additional changes you speak of. Introducing
codepaths that we don't exercise can lead to surprise failures later when we
do. That said, I suppose if you're able to give sufficient unit test
coverage, that's enough "exercise". I could go either way on this one.
(Sorry... I don't suppose that was very helpful, was it?)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand