(mynewt-core) 06/06: fs/fcb: Clean fcb_append inconsistency

[je...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jerzy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit f3bd5749bb3cbd5af04ad6149709b5d580f41b35
Author: Jerzy Kasenberg <je...@codecoup.pl>
AuthorDate: Wed Aug 7 13:57:17 2024 +0200

    fs/fcb: Clean fcb_append inconsistency
    
    Variable cnt and len were reused leading to some inconsistencies.
    
    1.
    cnt was number of bytes needed to store len but it was later
    changed to store number of bytes in flash to store len (that
    can be larger when flash alignment restrictions are applied).
    Later it was used to actually write len filed to flash, however
    if alignment was not 1 or 2 some random data would be written to
    flash (starting from tmp_str but extending to other values on stack).
    No there is separate variable to keep track how many bytes in
    flash are needed.
    
    2.
    calculating whether entry will fit in current sector,
    fcb_disk_area was not aligned correctly that could lead to
    problems for flash that has alignment that is higher then 8.
    
    3.
    append_loc was partially updated but fe_data_len was never filled
    leaving structure in inconsistent state.
    
    4.
    active->fe_data_len could have incorrect value (rounded up to alignment)
    This was small inconsistency that probably would not result in any
    problem
    
    Signed-off-by: Jerzy Kasenberg <je...@codecoup.pl>
---
 fs/fcb/src/fcb_append.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/fcb/src/fcb_append.c b/fs/fcb/src/fcb_append.c
index 444fda25b..ad804c0dc 100644
--- a/fs/fcb/src/fcb_append.c
+++ b/fs/fcb/src/fcb_append.c
@@ -86,24 +86,26 @@ fcb_append(struct fcb *fcb, uint16_t len, struct fcb_entry *append_loc)
     struct flash_area *fa;
     uint8_t tmp_str[2];
     int cnt;
+    int len_bytes_in_flash;
+    int entry_len_in_flash;
     int rc;
 
     cnt = fcb_put_len(tmp_str, len);
     if (cnt < 0) {
         return cnt;
     }
-    cnt = fcb_len_in_flash(fcb, cnt);
-    len = fcb_len_in_flash(fcb, len) + fcb_len_in_flash(fcb, FCB_CRC_SZ);
+    len_bytes_in_flash = fcb_len_in_flash(fcb, cnt);
+    entry_len_in_flash = fcb_entry_total_len(fcb, len);
 
     rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
     if (rc && rc != OS_NOT_STARTED) {
         return FCB_ERR_ARGS;
     }
     active = &fcb->f_active;
-    if (active->fe_elem_off + len + cnt > active->fe_area->fa_size) {
+    if (active->fe_elem_off + entry_len_in_flash > active->fe_area->fa_size) {
         fa = fcb_new_area(fcb, fcb->f_scratch_cnt);
         if (!fa || (fa->fa_size <
-            sizeof(struct fcb_disk_area) + len + cnt)) {
+                    fcb_len_in_flash(fcb, sizeof(struct fcb_disk_area)) + entry_len_in_flash)) {
             rc = FCB_ERR_NOSPACE;
             goto err;
         }
@@ -124,9 +126,10 @@ fcb_append(struct fcb *fcb, uint16_t len, struct fcb_entry *append_loc)
     }
     append_loc->fe_area = active->fe_area;
     append_loc->fe_elem_off = active->fe_elem_off;
-    append_loc->fe_data_off = active->fe_elem_off + cnt;
+    append_loc->fe_data_off = active->fe_elem_off + len_bytes_in_flash;
+    append_loc->fe_data_len = len;
 
-    active->fe_elem_off = append_loc->fe_data_off + len;
+    active->fe_elem_off = append_loc->fe_elem_off + entry_len_in_flash;
     active->fe_data_off = append_loc->fe_data_off;
     active->fe_data_len = len;