From 44c82bea9ea931ccd33978b672ce580651a507f9 Mon Sep 17 00:00:00 2001
From: David Anderson <davea@ssl.berkeley.edu>
Date: Mon, 4 May 2015 14:48:34 -0700
Subject: [PATCH] client: detect errors in directory enumeration

Previously, the dir_scan() function didn't distinguish between
- reaching the end of the directory
- errors
It just returned nonzero in either case.
This means that the function that cleans out a slot dir
(client_clean_out_dir())
could potentially return success even though the directory is nonempty.
This could potentially cause the recently-reported problem
where a slot dir contains a VM image from a previous job.
---
 client/app.cpp             | 20 +++++++++++++++-----
 client/app_control.cpp     |  2 +-
 client/sandbox.cpp         |  7 ++++++-
 clientgui/MainDocument.cpp |  2 +-
 lib/filesys.cpp            | 15 +++++++++++----
 5 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/client/app.cpp b/client/app.cpp
index b8dfefd79a..eaa52a56e0 100644
--- a/client/app.cpp
+++ b/client/app.cpp
@@ -553,22 +553,28 @@ bool ACTIVE_TASK_SET::is_slot_dir_in_use(char* dir) {
     return false;
 }
 
-// Get a free slot,
-// and make a slot dir if needed
+// Get a free slot:
+// either find an unused an empty slot dir,
+// or create a new slot dir if needed
 //
 int ACTIVE_TASK::get_free_slot(RESULT* rp) {
 #ifndef SIM
     int j, retval;
     char path[MAXPATHLEN];
 
+    // scan slot numbers: slots/0, slots/1, etc.
+    //
     for (j=0; ; j++) {
+        // skip slots that are in use by existing jobs
+        //
         if (gstate.active_tasks.is_slot_in_use(j)) continue;
 
-        // make sure we can make an empty directory for this slot
-        //
         get_slot_dir(j, path, sizeof(path));
         if (boinc_file_exists(path)) {
             if (is_dir(path)) {
+                // If the directory exists, try to clean it out.
+                // If this succeeds, use it.
+                //
                 retval = client_clean_out_dir(path, "get_free_slot()");
                 if (!retval) break;
                 if (log_flags.slot_debug) {
@@ -579,6 +585,8 @@ int ACTIVE_TASK::get_free_slot(RESULT* rp) {
                 }
             }
         } else {
+            // directory doesn't exist - create one
+            //
             retval = make_slot_dir(j);
             if (!retval) break;
         }
@@ -594,7 +602,9 @@ int ACTIVE_TASK::get_free_slot(RESULT* rp) {
     }
     slot = j;
     if (log_flags.slot_debug) {
-        msg_printf(rp->project, MSG_INFO, "[slot] assigning slot %d to %s", j, rp->name);
+        msg_printf(rp->project, MSG_INFO,
+            "[slot] assigning slot %d to %s", j, rp->name
+        );
     }
 #endif
     return 0;
diff --git a/client/app_control.cpp b/client/app_control.cpp
index 0add02e73d..b73bc91e81 100644
--- a/client/app_control.cpp
+++ b/client/app_control.cpp
@@ -805,7 +805,7 @@ bool ACTIVE_TASK::check_max_disk_exceeded() {
                 "Aborting task %s: exceeded disk limit: %.2fMB > %.2fMB\n",
                 result->name, disk_usage/MEGA, max_disk_usage/MEGA
             );
-            abort_task(EXIT_DISK_LIMIT_EXCEEDED, "Maximum disk usage exceeded");
+            abort_task(EXIT_DISK_LIMIT_EXCEEDED, "Disk usage limit exceeded");
             return true;
         }
     }
diff --git a/client/sandbox.cpp b/client/sandbox.cpp
index 3caa6b571a..661a7de9d5 100644
--- a/client/sandbox.cpp
+++ b/client/sandbox.cpp
@@ -312,7 +312,12 @@ int client_clean_out_dir(const char* dirpath, const char* reason) {
     while (1) {
         strcpy(filename, "");
         retval = dir_scan(filename, dirp, sizeof(filename));
-        if (retval) break;
+        if (retval) {
+            if (retval != ERR_NOT_FOUND) {
+                final_retval = retval;
+            }
+            break;
+        }
         sprintf(path, "%s/%s", dirpath,  filename);
         if (is_dir(path)) {
             retval = client_clean_out_dir(path, NULL);
diff --git a/clientgui/MainDocument.cpp b/clientgui/MainDocument.cpp
index fbf882df72..77357b70ef 100644
--- a/clientgui/MainDocument.cpp
+++ b/clientgui/MainDocument.cpp
@@ -2566,7 +2566,7 @@ wxString result_description(RESULT* result, bool show_resources) {
             strBuffer += _("Aborted: not started by deadline");
             break;
         case EXIT_DISK_LIMIT_EXCEEDED:
-            strBuffer += _("Aborted: disk limit exceeded");
+            strBuffer += _("Aborted: task disk limit exceeded");
             break;
         case EXIT_TIME_LIMIT_EXCEEDED:
             strBuffer += _("Aborted: run time limit exceeded");
diff --git a/lib/filesys.cpp b/lib/filesys.cpp
index 5f23ed3fec..4755e73572 100644
--- a/lib/filesys.cpp
+++ b/lib/filesys.cpp
@@ -149,7 +149,10 @@ DIRREF dir_open(const char* p) {
     return dirp;
 }
 
-// Scan through a directory and return the next file name in it
+// Scan through a directory and return:
+// 0 if an entry was found (the entry is returned in p)
+// ERR_NOT_FOUND if we reached the end of the directory
+// ERR_READDIR if some other error occurred
 //
 int dir_scan(char* p, DIRREF dirp, int p_len) {
 #ifdef _WIN32
@@ -175,9 +178,12 @@ int dir_scan(char* p, DIRREF dirp, int p_len) {
                 if (p) strlcpy(p, data.cFileName, p_len);
                 return 0;
             } else {
-                FindClose(dirp->handle);
+                DWORD ret = FindClose(dirp->handle);
                 dirp->handle = INVALID_HANDLE_VALUE;
-                return 1;
+                if (ret == ERROR_NO_MORE_FILES) {
+                    return ERR_NOT_FOUND;
+                }
+                return ERR_READDIR;
             }
         }
     }
@@ -190,7 +196,8 @@ int dir_scan(char* p, DIRREF dirp, int p_len) {
             if (p) strlcpy(p, dp->d_name, p_len);
             return 0;
         } else {
-            return ERR_READDIR;
+            if (errno) return ERR_READDIR;
+            return ERR_NOT_FOUND;
         }
     }
 #endif