You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Joerg Heinicke <jo...@gmx.de> on 2004/12/31 18:15:34 UTC
Re: svn commit: r123799 - /cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java
On 31.12.2004 16:12, antonio@apache.org wrote:
> Author: antonio
> Date: Fri Dec 31 07:12:59 2004
> New Revision: 123799
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=123799
> Log:
> Remove unused code
> Modified:
> cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java
>
> Modified: cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java
> Url: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java?view=diff&rev=123799&p1=cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java&r1=123798&p2=cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java&r2=123799
> ==============================================================================
> --- cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java (original)
> +++ cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java Fri Dec 31 07:12:59 2004
> @@ -226,17 +226,9 @@
> int status = STATUS_OK;
> final Iterator iter = ((TraversableSource) source).getChildren().iterator();
> while (iter.hasNext()) {
> - Source child = null;
> - try {
> - status = remove((Source) iter.next());
> - if (status != STATUS_OK) {
> - return status;
> - }
> - }
> - finally {
> - if (child != null) {
> - m_resolver.release(child);
------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^
> - }
> + status = remove((Source) iter.next());
> + if (status != STATUS_OK) {
> + return status;
> }
> }
> }
What about releasing the Source's??
Joerg
Please review this code (was: svn commit: r123799 -
/cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/co
coon/components/repository/SourceRepositoryImpl.java)
Posted by Antonio Gallardo <ag...@agssa.net>.
On Vie, 31 de Diciembre de 2004, 15:45, Joerg Heinicke dijo:
> On 31.12.2004 18:46, Antonio Gallardo wrote:
>
>>>> while (iter.hasNext()) {
>>>>- Source child = null;
>>>>- try {
>>>>- status = remove((Source) iter.next());
>>>>- if (status != STATUS_OK) {
>>>>- return status;
>>>>- }
>>>>- }
>>>>- finally {
>>>>- if (child != null) {
>>>>- m_resolver.release(child);
>>>
>>>------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>>
>>>>- }
>>>>+ status = remove((Source) iter.next());
>>>>+ if (status != STATUS_OK) {
>>>>+ return status;
>>>> }
>>>> }
>>>> }
>>>
>>>What about releasing the Source's??
>>
>>
>> 'child' is always = null. The condition never happen to be true. Then it
>> is unnecesary code.
>
> Ah, yes, from the code it does not behave differently than before. But I
> guess it has been buggy before too. It should probably read:
>
> while (iter.hasNext()) {
> Source child = null;
> try {
> child = (Source) iter.next();
> status = remove(child);
> if (status != STATUS_OK) {
> return status;
> }
> } finally {
> if (child != null) {
> m_resolver.release(child);
> }
> }
> }
>
> This was what I had in mind when claiming about the missing release of
> the sources. Maybe somebody else (the original author?) can review it.
Yep. I hope the original author or someone with more knowledge of this
class can give us his opinion. I was just cleaning the code ;-)
Best Regards,
Antonio Gallardo
>
> Joerg
>
Re: svn commit: r123799 - /cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/co
coon/components/repository/SourceRepositoryImpl.java
Posted by Joerg Heinicke <jo...@gmx.de>.
On 31.12.2004 18:46, Antonio Gallardo wrote:
>>> while (iter.hasNext()) {
>>>- Source child = null;
>>>- try {
>>>- status = remove((Source) iter.next());
>>>- if (status != STATUS_OK) {
>>>- return status;
>>>- }
>>>- }
>>>- finally {
>>>- if (child != null) {
>>>- m_resolver.release(child);
>>
>>------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>
>>>- }
>>>+ status = remove((Source) iter.next());
>>>+ if (status != STATUS_OK) {
>>>+ return status;
>>> }
>>> }
>>> }
>>
>>What about releasing the Source's??
>
>
> 'child' is always = null. The condition never happen to be true. Then it
> is unnecesary code.
Ah, yes, from the code it does not behave differently than before. But I
guess it has been buggy before too. It should probably read:
while (iter.hasNext()) {
Source child = null;
try {
child = (Source) iter.next();
status = remove(child);
if (status != STATUS_OK) {
return status;
}
} finally {
if (child != null) {
m_resolver.release(child);
}
}
}
This was what I had in mind when claiming about the missing release of
the sources. Maybe somebody else (the original author?) can review it.
Joerg
Re: svn commit: r123799 -
/cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/co
coon/components/repository/SourceRepositoryImpl.java
Posted by Antonio Gallardo <ag...@agssa.net>.
On Vie, 31 de Diciembre de 2004, 11:15, Joerg Heinicke dijo:
> On 31.12.2004 16:12, antonio@apache.org wrote:
>
>> Author: antonio
>> Date: Fri Dec 31 07:12:59 2004
>> New Revision: 123799
>>
>> URL: http://svn.apache.org/viewcvs?view=rev&rev=123799
>> Log:
>> Remove unused code
>> Modified:
>> cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java
>>
>> Modified:
>> cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java
>> Url:
>> http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java?view=diff&rev=123799&p1=cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java&r1=123798&p2=cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java&r2=123799
>> ==============================================================================
>> ---
>> cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java (original)
>> +++
>> cocoon/branches/BRANCH_2_1_X/src/blocks/repository/java/org/apache/cocoon/components/repository/SourceRepositoryImpl.java Fri
>> Dec 31 07:12:59 2004
>> @@ -226,17 +226,9 @@
>> int status = STATUS_OK;
>> final Iterator iter = ((TraversableSource)
>> source).getChildren().iterator();
>> while (iter.hasNext()) {
>> - Source child = null;
>> - try {
>> - status = remove((Source) iter.next());
>> - if (status != STATUS_OK) {
>> - return status;
>> - }
>> - }
>> - finally {
>> - if (child != null) {
>> - m_resolver.release(child);
>
> ------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>> - }
>> + status = remove((Source) iter.next());
>> + if (status != STATUS_OK) {
>> + return status;
>> }
>> }
>> }
>
> What about releasing the Source's??
'child' is always = null. The condition never happen to be true. Then it
is unnecesary code.
WDYT?
Best Regards,
Antonio Gallardo