You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Senthil Kumaran S <se...@collab.net> on 2009/08/28 10:55:45 UTC

Re: Fixing issue 3368

Stefan Sperling wrote:
>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
>> +            pval = svn_apr_hash_index_val(hi);
> 
> I don't have any comments on what the best way to fix the issue is,
> just one question on the code.
> 
> Why are you looping though all properties and test the very last one you end
> up with, instead of testing each property?

That is just a test. But that does not make sense, my final commit will have
proper checking.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388218

Re: Fixing issue 3368

Posted by Senthil Kumaran S <se...@collab.net>.
Stefan Sperling wrote:
> On Tue, Sep 15, 2009 at 07:39:07AM -0400, Mark Phippard wrote:
>> On Tue, Sep 15, 2009 at 6:55 AM, Senthil Kumaran S <se...@collab.net> wrote:
>>> Senthil Kumaran S wrote:
>>>> Stefan Sperling wrote:
>>>>>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
>>>>>> +            pval = svn_apr_hash_index_val(hi);
>>>>> I don't have any comments on what the best way to fix the issue is,
>>>>> just one question on the code.
>>>>>
>>>>> Why are you looping though all properties and test the very last one you end
>>>>> up with, instead of testing each property?
>>>> That is just a test. But that does not make sense, my final commit will have
>>>> proper checking.
>>> Committed a fix in r39329.
>> Great.  Any chance of nominating this for 1.6.x?  Or is it too tied up
>> with the changes on trunk?  A lot of people have complained about this
>> bug.
> 
> I just took a look at the commit, it's a very simple fix.
> Should be straightforward to backport.

Mark and Stefan, Nominated for backport to 1.6.x branch in r39331 (along with
the test case).

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395004

Re: Fixing issue 3368

Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Sep 15, 2009 at 7:59 AM, Stefan Sperling <st...@elego.de> wrote:
> On Tue, Sep 15, 2009 at 07:39:07AM -0400, Mark Phippard wrote:
>> On Tue, Sep 15, 2009 at 6:55 AM, Senthil Kumaran S <se...@collab.net> wrote:
>> > Senthil Kumaran S wrote:
>> >> Stefan Sperling wrote:
>> >>>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
>> >>>> +            pval = svn_apr_hash_index_val(hi);
>> >>> I don't have any comments on what the best way to fix the issue is,
>> >>> just one question on the code.
>> >>>
>> >>> Why are you looping though all properties and test the very last one you end
>> >>> up with, instead of testing each property?
>> >>
>> >> That is just a test. But that does not make sense, my final commit will have
>> >> proper checking.
>> >
>> > Committed a fix in r39329.
>>
>> Great.  Any chance of nominating this for 1.6.x?  Or is it too tied up
>> with the changes on trunk?  A lot of people have complained about this
>> bug.
>
> I just took a look at the commit, it's a very simple fix.
> Should be straightforward to backport.

Wow, I just looked at the commit too.  That took some good digging to
come up with such a simple, but not-obvious, fix.

Nice job Senthil.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395013

Re: Fixing issue 3368

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Sep 15, 2009 at 07:39:07AM -0400, Mark Phippard wrote:
> On Tue, Sep 15, 2009 at 6:55 AM, Senthil Kumaran S <se...@collab.net> wrote:
> > Senthil Kumaran S wrote:
> >> Stefan Sperling wrote:
> >>>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
> >>>> +            pval = svn_apr_hash_index_val(hi);
> >>> I don't have any comments on what the best way to fix the issue is,
> >>> just one question on the code.
> >>>
> >>> Why are you looping though all properties and test the very last one you end
> >>> up with, instead of testing each property?
> >>
> >> That is just a test. But that does not make sense, my final commit will have
> >> proper checking.
> >
> > Committed a fix in r39329.
> 
> Great.  Any chance of nominating this for 1.6.x?  Or is it too tied up
> with the changes on trunk?  A lot of people have complained about this
> bug.

I just took a look at the commit, it's a very simple fix.
Should be straightforward to backport.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395000

Re: Fixing issue 3368

Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Sep 15, 2009 at 6:55 AM, Senthil Kumaran S <se...@collab.net> wrote:
> Senthil Kumaran S wrote:
>> Stefan Sperling wrote:
>>>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
>>>> +            pval = svn_apr_hash_index_val(hi);
>>> I don't have any comments on what the best way to fix the issue is,
>>> just one question on the code.
>>>
>>> Why are you looping though all properties and test the very last one you end
>>> up with, instead of testing each property?
>>
>> That is just a test. But that does not make sense, my final commit will have
>> proper checking.
>
> Committed a fix in r39329.

Great.  Any chance of nominating this for 1.6.x?  Or is it too tied up
with the changes on trunk?  A lot of people have complained about this
bug.


-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394992

Re: Fixing issue 3368

Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
> Stefan Sperling wrote:
>>> +          for (hi = apr_hash_first(subpool, props); hi; hi = apr_hash_next(hi))
>>> +            pval = svn_apr_hash_index_val(hi);
>> I don't have any comments on what the best way to fix the issue is,
>> just one question on the code.
>>
>> Why are you looping though all properties and test the very last one you end
>> up with, instead of testing each property?
> 
> That is just a test. But that does not make sense, my final commit will have
> proper checking.

Committed a fix in r39329.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394963