Uploaded image for project: 'Commons JCI'
  1. Commons JCI
  2. JCI-67

Dubious use of mkdirs() return code

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1
    • Component/s: None
    • Labels:
      None

      Description

      FileRestoreStore.java uses mkdirs() as follows:

      final File parent = file.getParentFile();
      if (!parent.exists()) {
          if (!parent.mkdirs()) {
              throw new IOException("could not create" + parent);
          }
      }
      

      Now mkdirs() returns true only if the method actually created the directories; it's theoretically possible for the directory to be created in the window between the exists() and mkdirs() invocations.

      Also, the initial exists() call is redundant, because that's what mkdirs() does anyway (in the RI implementation, at least).

      I suggest the following instead:

      final File parent = file.getParentFile();
      if (!parent.mkdirs() && !parent.exists()) {
              throw new IOException("could not create" + parent);
          }
      }
      

      If mkdirs() returns false, the code then checks to see if the directory exists, so the throws clause will only be invoked if the parent really cannot be created.

      The same code also appears in AbstractTestCase and FilesystemAlterationMonitorTestCase.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                sebb@apache.org Sebb
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: