You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2015/01/07 00:27:00 UTC

[2/2] trafficserver git commit: Improve snapshot directory creation checks

Improve snapshot directory creation checks

Coverity CID #1021786
Coverity CID #1021785


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/70aec040
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/70aec040
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/70aec040

Branch: refs/heads/master
Commit: 70aec04064f6065beaa0575d05a931e179957e99
Parents: 82f9563
Author: James Peach <jp...@apache.org>
Authored: Mon Jan 5 09:43:25 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Tue Jan 6 15:22:45 2015 -0800

----------------------------------------------------------------------
 mgmt/FileManager.cc | 68 +++++++++++-------------------------------------
 1 file changed, 15 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/70aec040/mgmt/FileManager.cc
----------------------------------------------------------------------
diff --git a/mgmt/FileManager.cc b/mgmt/FileManager.cc
index 1508be3..df46719 100644
--- a/mgmt/FileManager.cc
+++ b/mgmt/FileManager.cc
@@ -61,11 +61,13 @@ FileManager::FileManager()
   ats_scoped_str snapshotDir(RecConfigReadSnapshotDir());
 
   // Check to see if the directory already exists, if not create it.
-  if (access(snapshotDir, F_OK) == -1) {
-    if (mkdir(snapshotDir, DIR_MODE) < 0) {
-      // Failed to create the snapshot directory
-      mgmt_fatal(stderr, 0, "[FileManager::FileManager] Failed to create the snapshot directory %s: %s\n", (const char *)snapshotDir, strerror(errno));
-    }
+  if (mkdir(snapshotDir, DIR_MODE) < 0 && errno != EEXIST) {
+    // Failed to create the snapshot directory
+    mgmt_fatal(stderr, 0, "[FileManager::FileManager] Failed to create the snapshot directory %s: %s\n", (const char *)snapshotDir, strerror(errno));
+  }
+
+  if (!ink_file_is_directory(snapshotDir)) {
+    mgmt_fatal(stderr, 0, "[FileManager::FileManager] snapshot directory %s is not a directory\n", (const char *)snapshotDir);
   }
 
   this->managedDir = snapshotDir.release();
@@ -107,12 +109,10 @@ FileManager::~FileManager()
 
   ink_hash_table_destroy(bindings);
 
-
   ink_mutex_destroy(&accessLock);
   ink_mutex_destroy(&cbListLock);
 }
 
-
 // void FileManager::registerCallback(FileCallbackFunc func)
 //
 //  Adds a new callback function
@@ -192,7 +192,6 @@ FileManager::fileChanged(const char *baseFileName, bool incVersion)
 
   ink_mutex_acquire(&cbListLock);
 
-
   for (cb = cblist.head; cb != NULL; cb = cb->link.next) {
     // Dup the string for each callback to be
     //  defensive incase it modified when it is not supposed to be
@@ -273,7 +272,6 @@ FileManager::doRollbackLocks(lockAction_t action)
   }
 }
 
-
 // void FileManager::abortRestore(const char* abortTo)
 //
 //  Iterates through the hash table of managed files
@@ -316,7 +314,6 @@ FileManager::abortRestore(const char *abortTo)
   }
 }
 
-
 // SnapResult FileManager::restoresSnap(const char* snapName)
 //
 //  Restores the snapshot with snapName.
@@ -385,9 +382,6 @@ FileManager::restoreSnap(const char *snapName, const char *snapDir)
   return result;
 }
 
-
-
-
 // SnapResult FileManager::removeSnap(const char* snapName)
 //
 //
@@ -451,14 +445,6 @@ FileManager::removeSnap(const char *snapName, const char *snapDir)
   return result;
 }
 
-
-
-
-
-
-
-
-
  //
  //  Creates a new snapshot with snapName
  //     Creates a directory named snapName in the snapshot directory
@@ -474,8 +460,6 @@ FileManager::takeSnap(const char *snapName, const char *snapDir)
   InkHashTableIteratorState iterator_state;
   char *snapPath;
   SnapResult callResult = SNAP_OK;
-  struct stat snapDirStat;
-
 
   // Make sure the user sent us a name
   if (snapName == NULL || *snapName == '\0') {
@@ -490,25 +474,22 @@ FileManager::takeSnap(const char *snapName, const char *snapDir)
     return SNAP_ILLEGAL_NAME;
   }
 
-
   snapPath = newPathString(snapDir, snapName);
 
-  if (!stat(snapPath, &snapDirStat)) {
-    if (!S_ISDIR(snapDirStat.st_mode)) {
-      delete[]snapPath;
-      return SNAP_DIR_CREATE_FAILED;
-    }
+  if (mkdir(snapPath, DIR_MODE) < 0 && errno != EEXIST) {
+    mgmt_log(stderr, "[FileManager::takeSnap] Failed to create directory for snapshot %s: %s\n", snapName, strerror(errno));
+    delete[]snapPath;
+    return SNAP_DIR_CREATE_FAILED;
   }
-  if (mkdir(snapPath, DIR_MODE) < 0) {
-    mgmt_log(stderr, "[FileManager::takeSnap] Failed to create directory for snapshot %s: %s\n",
-             snapName, strerror(errno));
+
+  if (!ink_file_is_directory(snapPath)) {
+    mgmt_log(stderr, "[FileManager::takeSnap] snapshot directory %s is not a directory\n", snapPath);
     delete[]snapPath;
     return SNAP_DIR_CREATE_FAILED;
   }
 
   ink_mutex_acquire(&accessLock);
 
-
   // To get a stable snap shot, we need to get the rollback
   //   locks on all configuration files so the files
   //   do not change from under us
@@ -521,7 +502,7 @@ FileManager::takeSnap(const char *snapName, const char *snapDir)
     rb = (Rollback *) ink_hash_table_entry_value(bindings, entry);
     callResult = this->copyFile(rb, snapPath);
     if (callResult != SNAP_OK) {
-      // Remove the failed napshot so that we do not have a partial
+      // Remove the failed snapshot so that we do not have a partial
       //   one hanging around
       if (removeSnap(snapName, snapDir) != SNAP_OK) {
         mgmt_log(stderr,
@@ -541,13 +522,6 @@ FileManager::takeSnap(const char *snapName, const char *snapDir)
   return callResult;
 }
 
-
-
-
-
-
-
-
 //
 //  SnapResult FileManager::readFile(const char* filePath, textBuffer* contents)
 //
@@ -631,12 +605,6 @@ FileManager::copyFile(Rollback * rb, const char *snapPath)
   return result;
 }
 
-
-
-
-
-
-
 // SnapResult FileManager::WalkSnaps(ExpandingArray* snapList)
 //
 //   Iterates through the snapshot directory and adds every snapshot
@@ -662,8 +630,6 @@ FileManager::WalkSnaps(ExpandingArray * snapList)
   return (SnapResult) r;
 }
 
-
-
 // void FileManger::rereadConfig()
 //
 //   Interates through the list of managed files and
@@ -742,8 +708,6 @@ FileManager::displaySnapOption(textBuffer * output)
   }
 }
 
-
-
 // void FileManger::createSelect(char* formVar, textBuffer* output, ExpandingArray*)
 //
 //  Creats a form with a select list.  The select options come
@@ -775,7 +739,6 @@ FileManager::createSelect(char *action, textBuffer * output, ExpandingArray * op
   }
 }
 
-
 // bool checkValidName(const char* name)
 //
 // if the string is invalid, ie. all white spaces or contains "irregular" chars,
@@ -796,7 +759,6 @@ FileManager::checkValidName(const char *name)
   return false;                 // all white spaces
 }
 
-
 //  int snapEntryCmpFunc(void* e1, void* e2)
 //
 //  a cmp function for snapshot structs that can