[GitHub] [mynewt-core] kasjer commented on a diff in pull request #2949: fs/fcb2: fix endless loop in fcb2_getprev if a flash error occurs

["kasjer (via GitHub)" <gi...@apache.org>]

kasjer commented on code in PR #2949:
URL: https://github.com/apache/mynewt-core/pull/2949#discussion_r1118417398


##########
fs/fcb2/src/fcb_getprev.c:
##########
@@ -52,7 +52,7 @@ fcb2_sector_find_last(struct fcb2 *fcb, struct fcb2_entry *loc)
             }
         }
         loc->fe_entry_num++;
-    } while (1);
+    } while (rc == 0);

Review Comment:
   Endless loop was supposed to handle cases where FCB2 had corrupted entries (most likely CRC for broken writes).
   I guess then in your case, due to some flash read error, loop would not finish.
   FCB2  sector starts with header `fcb2_disk_area`; data is stored in flash after the header starting from the beginning while entries holding CRC and data position and length are stored starting from the end of the sector.
   When read error occurs fe_entry_num should be increased like it is right now but some sanity check should be performed to see if probably entry does not reach to occupied space or header if read error persist.
   I guess similar check is done when new data is being written to FCB and there is not space for data+entry.
   Proposed solution does not cover case where there are some errors in flash that could be recovered.



##########
fs/fcb2/src/fcb_getprev.c:
##########
@@ -34,7 +34,7 @@ fcb2_sector_find_last(struct fcb2 *fcb, struct fcb2_entry *loc)
         if (rc == 0) {
             last_valid = loc->fe_entry_num;
         }
-        if (rc == FCB2_ERR_NOVAR) {
+        else if (rc == FCB2_ERR_NOVAR) {

Review Comment:
   else should be in previous line to keep coding style presistant



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org