Be careful resetting flag bits

Only reset those that are set, do not assume that 0 -> 1 writes are ignored.

(hopefully) fixes https://github.com/pellepl/spiffs/issues/172

.travis.yml

@@ -5,4 +5,7 @@ compiler:
 
 before_script:
 
-script: make all && make clean && make test && make build-all && make clean test FLAGS=-DSPIFFS_OBJ_META_LEN=8
+script: >
+  make all && make clean && make test && make build-all &&
+  make clean test FLAGS=-DSPIFFS_OBJ_META_LEN=8 &&
+  make clean test FLAGS=-DSPIFFS_NO_BLIND_WRITES=1

src/default/spiffs_config.h

@@ -311,6 +311,17 @@
 #define SPIFFS_IX_MAP                         1
 #endif
 
+// By default SPIFFS in some cases relies on the property of NOR flash that bits
+// cannot be set from 0 to 1 by writing and that controllers will ignore such
+// bit changes. This results in fewer reads as SPIFFS can in some cases perform
+// blind writes, with all bits set to 1 and only those it needs reset set to 0.
+// Most of the chips and controllers allow this behavior, so the default is to
+// use this technique. If your controller is one of the rare ones that don't,
+// turn this option on and SPIFFS will perform a read-modify-write instead.
+#ifndef SPIFFS_NO_BLIND_WRITES
+#define SPIFFS_NO_BLIND_WRITES                0
+#endif
+
 // Set SPIFFS_TEST_VISUALISATION to non-zero to enable SPIFFS_vis function
 // in the api. This function will visualize all filesystem using given printf
 // function.

src/spiffs_check.c

@@ -161,11 +161,17 @@ static s32_t spiffs_delete_obj_lazy(spiffs *fs, spiffs_obj_id obj_id) {
     return SPIFFS_OK;
   }
   SPIFFS_CHECK_RES(res);
-  u8_t flags = 0xff & ~SPIFFS_PH_FLAG_IXDELE;
+  u8_t flags = 0xff;
+#if SPIFFS_NO_BLIND_WRITES
+  res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_LU | SPIFFS_OP_C_READ,
+      0, SPIFFS_PAGE_TO_PADDR(fs, objix_hdr_pix) + offsetof(spiffs_page_header, flags),
+      sizeof(flags), &flags);
+  SPIFFS_CHECK_RES(res);
+#endif
+  flags &= ~SPIFFS_PH_FLAG_IXDELE;
   res = _spiffs_wr(fs, SPIFFS_OP_T_OBJ_LU | SPIFFS_OP_C_UPDT,
       0, SPIFFS_PAGE_TO_PADDR(fs, objix_hdr_pix) + offsetof(spiffs_page_header, flags),
-      sizeof(u8_t),
-      (u8_t *)&flags);
+      sizeof(flags), &flags);
   return res;
 }
 
@@ -423,10 +429,17 @@ static s32_t spiffs_lookup_check_validate(spiffs *fs, spiffs_obj_id lu_obj_id, s
           // just finalize
           SPIFFS_CHECK_DBG("LU: FIXUP: unfinalized page is referred, finalizing\n");
           CHECK_CB(fs, SPIFFS_CHECK_LOOKUP, SPIFFS_CHECK_FIX_LOOKUP, p_hdr->obj_id, p_hdr->span_ix);
-          u8_t flags = 0xff & ~SPIFFS_PH_FLAG_FINAL;
+          u8_t flags = 0xff;
+#if SPIFFS_NO_BLIND_WRITES
+          res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_DA | SPIFFS_OP_C_READ,
+              0, SPIFFS_PAGE_TO_PADDR(fs, cur_pix) + offsetof(spiffs_page_header, flags),
+              sizeof(flags), &flags);
+          SPIFFS_CHECK_RES(res);
+#endif
+          flags &= ~SPIFFS_PH_FLAG_FINAL;
           res = _spiffs_wr(fs, SPIFFS_OP_T_OBJ_DA | SPIFFS_OP_C_UPDT,
               0, SPIFFS_PAGE_TO_PADDR(fs, cur_pix) + offsetof(spiffs_page_header, flags),
-              sizeof(u8_t), (u8_t*)&flags);
+              sizeof(flags), &flags);
         }
       }
     }

src/spiffs_nucleus.c

@@ -877,8 +877,6 @@ s32_t spiffs_page_delete(
     spiffs *fs,
     spiffs_page_ix pix) {
   s32_t res;
-  spiffs_page_header hdr;
-  hdr.flags = 0xff & ~(SPIFFS_PH_FLAG_DELET | SPIFFS_PH_FLAG_USED);
   // mark deleted entry in source object lookup
   spiffs_obj_id d_obj_id = SPIFFS_OBJ_ID_DELETED;
   res = _spiffs_wr(fs, SPIFFS_OP_T_OBJ_LU | SPIFFS_OP_C_DELE,
@@ -892,11 +890,18 @@ s32_t spiffs_page_delete(
   fs->stats_p_allocated--;
 
   // mark deleted in source page
+  u8_t flags = 0xff;
+#if SPIFFS_NO_BLIND_WRITES
+  res = _spiffs_rd(fs, SPIFFS_OP_T_OBJ_DA | SPIFFS_OP_C_READ,
+      0, SPIFFS_PAGE_TO_PADDR(fs, pix) + offsetof(spiffs_page_header, flags),
+      sizeof(flags), &flags);
+  SPIFFS_CHECK_RES(res);
+#endif
+  flags &= ~(SPIFFS_PH_FLAG_DELET | SPIFFS_PH_FLAG_USED);
   res = _spiffs_wr(fs, SPIFFS_OP_T_OBJ_DA | SPIFFS_OP_C_DELE,
       0,
       SPIFFS_PAGE_TO_PADDR(fs, pix) + offsetof(spiffs_page_header, flags),
-      sizeof(u8_t),
-      (u8_t *)&hdr.flags);
+      sizeof(flags), &flags);
 
   return res;
 }

src/test/test_spiffs.c

@@ -200,14 +200,17 @@ static s32_t _write(
   }
 
   for (i = 0; i < size; i++) {
-    if (((addr + i) & (SPIFFS_CFG_LOG_PAGE_SZ(&__fs)-1)) != offsetof(spiffs_page_header, flags)) {
-      if (check_valid_flash && ((AREA(addr + i) ^ src[i]) & src[i])) {
-        printf("trying to write %02x to %02x at addr %08x (as part of writing %d bytes to addr %08x)\n", src[i], AREA(addr + i), addr+i, size, addr);
-        spiffs_page_ix pix = (addr + i) / SPIFFS_CFG_LOG_PAGE_SZ(&__fs);
-        dump_page(&__fs, pix);
-	ERREXIT();
-        return -1;
-      }
+#if !SPIFFS_NO_BLIND_WRITES
+    if (((addr + i) & (SPIFFS_CFG_LOG_PAGE_SZ(&__fs)-1)) == offsetof(spiffs_page_header, flags)) {
+      /* Blind flag writes are allowed. */
+    } else
+#endif
+    if (check_valid_flash && ((AREA(addr + i) ^ src[i]) & src[i])) {
+      printf("trying to write %02x to %02x at addr %08x (as part of writing %d bytes to addr %08x)\n", src[i], AREA(addr + i), addr+i, size, addr);
+      spiffs_page_ix pix = (addr + i) / SPIFFS_CFG_LOG_PAGE_SZ(&__fs);
+      dump_page(&__fs, pix);
+      ERREXIT();
+      return -1;
     }
     AREA(addr + i) &= src[i];
   }