Browse Source

Moves file detection to separate findNextImageAfter() function.

This also uses some additional buffers to make the code easier to follow (hopefully) and help protect against excessively long paths.
saybur 2 years ago
parent
commit
893a841c4e
1 changed files with 94 additions and 77 deletions
  1. 94 77
      src/BlueSCSI_disk.cpp

+ 94 - 77
src/BlueSCSI_disk.cpp

@@ -540,6 +540,82 @@ static void scsiDiskLoadConfig(int target_idx, const char *section)
     }
 }
 
+// Finds filename with the lowest lexical order _after_ the given filename in
+// the given folder. If there is no file after the given one, or if there is
+// no current file, this will return the lowest filename encountered.
+static int findNextImageAfter(image_config_t &img,
+        const char* dirname, const char* filename,
+        char* buf, size_t buflen)
+{
+    FsFile dir;
+    if (dirname[0] == '\0')
+    {
+        log("Image directory name invalid for ID", (img.scsiId & 7));
+        return 0;
+    }
+    if (!dir.open(dirname))
+    {
+        log("Image directory '", dirname, "' couldn't be opened");
+    }
+    if (!dir.isDir())
+    {
+        log("Can't find images in '", dirname, "', not a directory");
+        dir.close();
+        return 0;
+    }
+
+    char first_name[MAX_FILE_PATH] = {'\0'};
+    char candidate_name[MAX_FILE_PATH] = {'\0'};
+    FsFile file;
+    while (file.openNext(&dir, O_RDONLY))
+    {
+        if (file.isDir()) continue;
+        if (!file.getName(buf, MAX_FILE_PATH))
+        {
+            log("Image directory '", dirname, "'had invalid file");
+            continue;
+        }
+        if (!scsiDiskFilenameValid(buf)) continue;
+
+        // keep track of the first item to allow wrapping
+        // without having to iterate again
+        if (first_name[0] == '\0' || strcmp(buf, first_name) < 0)
+        {
+            strncpy(first_name, buf, sizeof(first_name));
+        }
+
+        // discard if no selected name, or if candidate is before (or is) selected
+        if (filename[0] == '\0' || strcmp(buf, filename) <= 0) continue;
+
+        // if we got this far and the candidate is either 1) not set, or 2) is a
+        // lower item than what has been encountered thus far, it is the best choice
+        if (candidate_name[0] == '\0' || strcmp(buf, candidate_name) < 0)
+        {
+            strncpy(candidate_name, buf, sizeof(candidate_name));
+        }
+    }
+
+    if (candidate_name[0] != '\0')
+    {
+        img.image_index++;
+        strncpy(img.current_image, candidate_name, sizeof(img.current_image));
+        strncpy(buf, candidate_name, buflen);
+        return strlen(candidate_name);
+    }
+    else if (first_name[0] != '\0')
+    {
+        img.image_index = 0;
+        strncpy(img.current_image, first_name, sizeof(img.current_image));
+        strncpy(buf, first_name, buflen);
+        return strlen(first_name);
+    }
+    else
+    {
+        log("Image directory '", dirname, "' was empty");
+        return 0;
+    }
+}
+
 int scsiDiskGetNextImageName(image_config_t &img, char *buf, size_t buflen)
 {
     int target_idx = img.scsiId & 7;
@@ -556,97 +632,38 @@ int scsiDiskGetNextImageName(image_config_t &img, char *buf, size_t buflen)
     if (img.image_directory)
     {
         // image directory was found during startup
-        FsFile dir;
+        char dirname[MAX_FILE_PATH];
         char key[] = "ImgDir";
-        ini_gets(section, key, "", buf, buflen, CONFIGFILE);
-        if (buf[0] != '\0')
-        {
-            if (!dir.open(buf))
-            {
-                log("ImgDir '", buf, "' couldn't be opened.");
-            }
-            if (!dir.isDir())
-            {
-                log("ImgDir '", buf, "' is not a directory.");
-                dir.close();
-                return false;
-            }
-        }
-        else
+        int dirlen = ini_gets(section, key, "", dirname, sizeof(dirname), CONFIGFILE);
+        if (!dirlen)
         {
             // If image_directory set but ImageDir is not, could be used to
             // indicate an image directory configured via folder structure.
             // Not implemented, so treat this as equivalent to missing ImageDir
-            return false;
-        }
-
-        // Find the filename with the lowest lexical order _after_ the
-        // currently selected file. If there is none, or if there is no
-        // current file, use the lowest filename encountered.
-        char first_name[MAX_FILE_PATH] = {'\0'};
-        char candidate_name[MAX_FILE_PATH] = {'\0'};
-        FsFile file;
-        while (file.openNext(&dir, O_RDONLY))
-        {
-            if (file.isDir()) continue;
-            if (!file.getName(buf, MAX_FILE_PATH))
-            {
-                log("Image directory had invalid file for ID", target_idx);
-                continue;
-            }
-            if (!scsiDiskFilenameValid(buf)) continue;
-
-            // keep track of the first item to allow wrapping
-            // without having to iterate again
-            if (first_name[0] == '\0' || strcmp(buf, first_name) < 0)
-            {
-                strncpy(first_name, buf, sizeof(first_name));
-            }
-
-            // discard if no selected name, or if candidate is before (or is) selected
-            if (img.current_image[0] == '\0' || strcmp(buf, img.current_image) <= 0) continue;
-
-            // if we got this far and the candidate is either 1) not set, or 2) is a
-            // lower item than what has been encountered thus far, it is the best choice
-            if (candidate_name[0] == '\0' || strcmp(buf, candidate_name) < 0)
-            {
-                strncpy(candidate_name, buf, sizeof(candidate_name));
-            }
+            return 0;
         }
 
-        // create base path for the result
-        // *TODO* this needs handling for leading and trailing slashes
-        int dirlen = dir.getName(buf, buflen);
-        dir.close(); // done with this
-        if (dirlen > 0 && dirlen - 1 < buflen)
-        {
-            buf[dirlen] = '/';
-            dirlen++;
-        }
-        else
-        {
-            log("Image directory name invalid for ID", target_idx);
-        }
+        // find the next filename
+        char nextname[MAX_FILE_PATH];
+        int nextlen = findNextImageAfter(img, dirname, img.current_image, nextname, sizeof(nextname));
 
-        // append the filename
-        if (candidate_name[0] != '\0')
+        if (nextlen == 0)
         {
-            strncpy(img.current_image, candidate_name, sizeof(img.current_image));
-            strncpy(buf + dirlen, candidate_name, buflen - dirlen);
-            img.image_index++;
-            return dirlen + strlen(candidate_name);
+            log("Image directory was empty for ID", target_idx);
+            return 0;
         }
-        else if (first_name[0] != '\0')
+        else if (buflen < nextlen + dirlen + 1)
         {
-            strncpy(img.current_image, first_name, sizeof(img.current_image));
-            strncpy(buf + dirlen, first_name, buflen - dirlen);
-            img.image_index = 0;
-            return dirlen + strlen(first_name);
+            log("Directory '", dirname, "' and file '", nextname, "' exceed allowed length");
+            return 0;
         }
         else
         {
-            log("Image directory was empty for ID", target_idx);
-            return 0;
+            // construct a return value
+            strncpy(buf, dirname, buflen);
+            buf[dirlen] = '/';
+            strncpy(buf + dirlen + 1, nextname, buflen - dirlen - 1);
+            return dirlen + nextlen;
         }
     }
     else