qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marnie McCormack <marnie.mccorm...@googlemail.com>
Subject Re: svn commit: r750946 - /qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java
Date Mon, 09 Mar 2009 08:55:12 GMT
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.

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
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message