qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Ritchie <ritch...@apache.org>
Subject Re: svn commit: r750946 - /qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
Date Mon, 09 Mar 2009 16:18:16 GMT
2009/3/9 Marnie McCormack <marnie.mccormack@googlemail.com>:
> Hi Martin,
>
> In general terms I would think this isn't a great thing - it could, for
> example, indicate that the file the code is trying to delete never
> existed/is incorrect so the calling code needs to care about what happened
> if the delete fails for that reason.
>
> I'd suggest that this method should throw the exceptions rather than return
> a boolean, so the calling code can handle as appropriate.

I agree, was trying to fix a problem the wrong way. I have reverted
the changes and FU.delete() will now behave as you would expect with
java.io.File. I think the null values that I was seeing being passed
in were an artefact of the other failure I was seeing.

Martin

> Regards,
> Marnie
>
> On Fri, Mar 6, 2009 at 3:46 PM, <ritchiem@apache.org> wrote:
>
>> Author: ritchiem
>> Date: Fri Mar  6 15:46:14 2009
>> New Revision: 750946
>>
>> URL: http://svn.apache.org/viewvc?rev=750946&view=rev
>> Log:
>> FileUtils : Was not correctly handling the case where a File object became
>> null, it would previously have thrown a NPE which was erroneously caught
>> this and declared the delete to have failed. If there is nothing to delete
>> (signified by the Null File object) then the delete should pass.
>>
>> Modified:
>>
>>  qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
>>
>> Modified:
>> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
>> URL:
>> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java?rev=750946&r1=750945&r2=750946&view=diff
>>
>> ==============================================================================
>> ---
>> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
>> (original)
>> +++
>> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
>> Fri Mar  6 15:46:14 2009
>> @@ -246,18 +246,19 @@
>>      {
>>          boolean success = true;
>>
>> +         // If we have nothing to delete then it must be ok to say it was
>> deleted.
>> +         if (file == null)
>> +         {
>> +             return true;
>> +         }
>> +
>>          if (file.isDirectory())
>>          {
>>              if (recursive)
>>              {
>> -                 try{
>> -                     for (File subFile : file.listFiles())
>> -                     {
>> -                         success = delete(subFile, true) & success ;
>> -                     }
>> -                 }catch (NullPointerException npe)
>> +                 for (File subFile : file.listFiles())
>>                  {
>> -                     success = false;
>> +                     success = delete(subFile, true) & success ;
>>                  }
>>
>>                  return success && file.delete();
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project:      http://qpid.apache.org
>> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>>
>>
>



-- 
Martin Ritchie

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Mime
View raw message