SdFat -> FS HAL mode fixes & test (#8833)

* re-use SdFat access mode through static const, no need to hard-code our own value w/ cast in the macro * separate read-modes from flags; read, write and rw are distinct numbers * simple compile-time tests in .cpp resolve #8831
esp8266 · Jan 31, 2023 · 6dfebec · 6dfebec
1 parent d7da591
commit 6dfebec
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 16 deletions.
diff --git a/libraries/SD/src/SD.cpp b/libraries/SD/src/SD.cpp
@@ -1,5 +1,14 @@
 #include "SD.h"
 
+static_assert(__builtin_strcmp(SDClassFileMode(FILE_READ), "r") == 0, "");
+static_assert(__builtin_strcmp(SDClassFileMode(FILE_WRITE), "a+") == 0, "");
+
+static_assert(__builtin_strcmp(SDClassFileMode(O_RDONLY), "r") == 0, "");
+static_assert(__builtin_strcmp(SDClassFileMode(O_WRONLY), "w+") == 0, "");
+static_assert(__builtin_strcmp(SDClassFileMode(O_RDWR), "w+") == 0, "");
+static_assert(__builtin_strcmp(SDClassFileMode(O_WRONLY | O_APPEND), "a") == 0, "");
+static_assert(__builtin_strcmp(SDClassFileMode(O_RDWR | O_APPEND), "a+") == 0, "");
+
 #if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_SD)
 SDClass SD;
 #endif

diff --git a/libraries/SD/src/SD.h b/libraries/SD/src/SD.h
@@ -24,11 +24,44 @@
 #include <FS.h>
 #include <SDFS.h>
 
+// Avoid type ambiguity, force u8 instead of untyped literal
+// ref. #6106 as to why we add APPEND to WRITE
+
+inline constexpr uint8_t SDClassFileRead { FILE_READ };
 #undef FILE_READ
-#define FILE_READ ((uint8_t)O_READ)
-#undef FILE_WRITE
-#define FILE_WRITE ((uint8_t)(O_READ | O_WRITE | O_CREAT | O_APPEND))
+#define FILE_READ SDClassFileRead
 
+inline constexpr uint8_t SDClassFileWrite { FILE_WRITE | O_APPEND };
+#undef FILE_WRITE
+#define FILE_WRITE SDClassFileWrite
+
+static inline constexpr const char* SDClassFileMode(uint8_t mode) {
+    bool read = false;
+    bool write = false;
+
+    switch (mode & O_ACCMODE) {
+    case O_RDONLY:
+        read = true;
+        break;
+    case O_WRONLY:
+        write = true;
+        break;
+    case O_RDWR:
+        read = true;
+        write = true;
+        break;
+    }
+
+    const bool append = (mode & O_APPEND) > 0;
+
+    if      (  read && !write )            { return "r";  }
+    else if ( !read &&  write && !append ) { return "w+"; }
+    else if ( !read &&  write &&  append ) { return "a";  }
+    else if (  read &&  write && !append ) { return "w+"; } // may be a bug in FS::mode interpretation, "r+" seems proper
+    else if (  read &&  write &&  append ) { return "a+"; }
+
+    return "r";
+}
 
 class SDClass {
 public:
@@ -45,7 +78,7 @@ class SDClass {
     }
 
     fs::File open(const char *filename, uint8_t mode = FILE_READ) {
-        return SDFS.open(filename, getMode(mode));
+        return SDFS.open(filename, SDClassFileMode(mode));
     }
 
     fs::File open(const char *filename, const char *mode) {
@@ -158,18 +191,6 @@ class SDClass {
     }
 
 private:
-    const char *getMode(uint8_t mode) {
-        bool read = (mode & O_READ) ? true : false;
-        bool write = (mode & O_WRITE) ? true : false;
-        bool append = (mode & O_APPEND) ? true : false;
-        if      (  read & !write )           { return "r";  }
-        else if ( !read &  write & !append ) { return "w+"; }
-        else if ( !read &  write &  append ) { return "a";  }
-        else if (  read &  write & !append ) { return "w+"; } // may be a bug in FS::mode interpretation, "r+" seems proper
-        else if (  read &  write &  append ) { return "a+"; }
-        else                                 { return "r";  }
-    }
-
     static time_t wrapperTimeCB(void) {
         extern void (*__SD__userDateTimeCB)(uint16_t*, uint16_t*);
         if (__SD__userDateTimeCB) {