vrahane commented on code in PR #3365:
URL: https://github.com/apache/mynewt-core/pull/3365#discussion_r1956980225


##########
sys/log/full/src/log_fcb_bmark.c:
##########
@@ -70,65 +121,247 @@ log_fcb_clear_bmarks(struct fcb_log *fcb_log)
 {
     fcb_log->fl_bset.lfs_size = 0;
     fcb_log->fl_bset.lfs_next = 0;
+    fcb_log->fl_bset.lfs_next_non_s = 0;
+    fcb_log->fl_bset.lfs_non_s_size = 0;
+    memset(fcb_log->fl_bset.lfs_bmarks, 0,
+           sizeof(struct log_fcb_bmark) *
+           fcb_log->fl_bset.lfs_cap);
 }
 
-const struct log_fcb_bmark *
-log_fcb_closest_bmark(const struct fcb_log *fcb_log, uint32_t index)
+struct log_fcb_bmark *
+log_fcb_get_bmarks(struct log *log, uint32_t *bmarks_size)
 {
-    const struct log_fcb_bmark *closest;
-    const struct log_fcb_bmark *bmark;
-    uint32_t min_diff;
+    struct fcb_log *fcb_log = (struct fcb_log *)log->l_arg;
+
+    *bmarks_size = fcb_log->fl_bset.lfs_cap;
+
+    return fcb_log->fl_bset.lfs_bmarks;
+}
+
+struct log_fcb_bmark *
+log_fcb_closest_bmark(struct fcb_log *fcb_log, uint32_t index,
+                      int *min_diff)
+{
+    struct log_fcb_bmark *closest;
+    struct log_fcb_bmark *bmark;
     uint32_t diff;
     int i;
 
-    min_diff = UINT32_MAX;
+    *min_diff = -1;
     closest = NULL;
 
+    /* This works for both sector as well as non-sector bmarks
+     * because we calculate the min diff and iterate to the end
+     * of the bmarks array keeping track of min diff
+     */
     for (i = 0; i < fcb_log->fl_bset.lfs_size; i++) {
         bmark = &fcb_log->fl_bset.lfs_bmarks[i];
+#if MYNEWT_VAL(LOG_FCB)
+        if (!fcb_log->fl_bset.lfs_bmarks[i].lfb_entry.fe_area) {
+            /* Previous closest bookmark is the closest one */
+            break;
+        }
+#elif MYNEWT_VAL(LOG_FCB2)
+        if (!fcb_log->fl_bset.lfs_bmarks[i].lfb_entry.fe_range) {
+            /* Previous closest bookmark is the closest one */
+            break;
+        }
+#endif
         if (bmark->lfb_index <= index) {
             diff = index - bmark->lfb_index;
-            if (diff < min_diff) {
-                min_diff = diff;
+            if (diff < *min_diff) {
+                *min_diff = diff;
                 closest = bmark;
+                MODLOG_INFO(LOG_MODULE_DEFAULT, "index: %u, closest bmark idx: 
%u, \n",
+                            (unsigned int)index,
+                            (unsigned int)bmark->lfb_index);
+                /* We found the exact match, no need to keep searching for a
+                 * better match
+                 */
+                if (*min_diff == 0) {
+                    break;
+                }
             }
         }
     }
 
     return closest;
 }
 
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
 #if MYNEWT_VAL(LOG_FCB)
-void
-log_fcb_add_bmark(struct fcb_log *fcb_log, const struct fcb_entry *entry,
-                  uint32_t index)
+static void
+log_fcb_insert_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
+                     uint32_t index, bool sect_bmark, int pos)
 #elif MYNEWT_VAL(LOG_FCB2)
-void
-log_fcb_add_bmark(struct fcb_log *fcb_log, const struct fcb2_entry *entry,
-                  uint32_t index)
+static void
+log_fcb_insert_bmark(struct fcb_log *fcb_log, fcb2_entry *entry,
+                     uint32_t index, bool sect_bmark, int pos)
 #endif
 {
     struct log_fcb_bset *bset;
 
     bset = &fcb_log->fl_bset;
 
-    if (bset->lfs_cap == 0) {
-        return;
+    if (pos == bset->lfs_cap - 1) {
+        goto add;
     }
 
-    bset->lfs_bmarks[bset->lfs_next] = (struct log_fcb_bmark) {
+    memcpy(&bset->lfs_bmarks[pos + 1], &bset->lfs_bmarks[pos],
+           sizeof(bset->lfs_bmarks[0]) * bset->lfs_size - pos - 1);
+
+add:
+    bset->lfs_bmarks[pos] = (struct log_fcb_bmark) {
         .lfb_entry = *entry,
         .lfb_index = index,
+        .lfb_sect_bmark = sect_bmark
     };
 
     if (bset->lfs_size < bset->lfs_cap) {
         bset->lfs_size++;
     }
 
     bset->lfs_next++;
-    if (bset->lfs_next >= bset->lfs_cap) {
-        bset->lfs_next = 0;
+    bset->lfs_next %= bset->lfs_cap;
+}
+#endif
+
+#if MYNEWT_VAL(LOG_FCB)
+static void
+log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
+                      uint32_t index, bool sect_bmark, int pos)
+#elif MYNEWT_VAL(LOG_FCB2)
+static void
+log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
+                      uint32_t index, bool sect_bmark, int pos)
+#endif
+{
+    int i = 0;
+    struct log_fcb_bset *bset = &fcb_log->fl_bset;
+    bool en_sect_bmark = sect_bmark & bset->lfs_en_sect_bmarks;
+
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+    if (en_sect_bmark) {
+        for (i = fcb_log->fl_fcb.f_sector_cnt;
+             i < (bset->lfs_non_s_size + fcb_log->fl_fcb.f_sector_cnt);
+             i++) {
+            if (index == bset->lfs_bmarks[i].lfb_index) {
+                /* If index matches, no need to replace */
+                return;
+            }
+        }
+    } else
+#endif
+    if (!en_sect_bmark) {
+        for (i = 0; i < bset->lfs_non_s_size; i++) {
+            if (index == bset->lfs_bmarks[i].lfb_index) {
+                /* If index matches, no need to replace */
+                return;
+            }
+        }
     }
+
+    bset->lfs_bmarks[pos] = (struct log_fcb_bmark) {
+        .lfb_entry = *entry,
+        .lfb_index = index,
+        .lfb_sect_bmark = sect_bmark
+    };
+}
+
+#if MYNEWT_VAL(LOG_FCB)
+void
+log_fcb_add_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
+                  uint32_t index, bool sect_bmark)
+#elif MYNEWT_VAL(LOG_FCB2)
+void
+log_fcb_add_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
+                  uint32_t index, bool sect_bmark)
+#endif
+{
+    struct log_fcb_bset *bset = &fcb_log->fl_bset;
+    int i = 0;
+    int pos = 0;
+    bool en_sect_bmark = sect_bmark & bset->lfs_en_sect_bmarks;
+
+    (void)i;
+
+    if (bset->lfs_cap == 0) {
+        return;
+    }
+
+    (void)en_sect_bmark;
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+    if (en_sect_bmark) {
+        for (i = 0; i < bset->lfs_size; i++) {
+            if (index > bset->lfs_bmarks[i].lfb_index) {
+                pos = i;
+                break;
+            } else {
+                pos = i + 1;
+            }
+        }
+        log_fcb_insert_bmark(fcb_log, entry, index, sect_bmark, pos);
+        if (pos >= fcb_log->fl_fcb.f_sector_cnt - 1) {
+            /* Make sure inserts do not trickle into the non-sector
+             * bookmarks
+             */
+            memset(&bset->lfs_bmarks[pos], 0, sizeof(bset->lfs_bmarks[0]));
+        } else {
+            MODLOG_DEBUG(LOG_MODULE_DEFAULT, "insert bmark index: %u, pos: %u, 
\n",
+                         (unsigned int)index,
+                         (unsigned int)pos);
+        }
+    } else {
+        /* Replace oldest non-sector bmark */
+        log_fcb_replace_bmark(fcb_log, entry, index, sect_bmark,
+                              bset->lfs_next_non_s +
+                              bset->lfs_en_sect_bmarks ? 
fcb_log->fl_fcb.f_sector_cnt
+                              : 0);
+        MODLOG_DEBUG(LOG_MODULE_DEFAULT, "replace bmark index: %u, pos: %u, 
\n",
+                     (unsigned int)index,
+                     (unsigned int)bset->lfs_next_non_s + 
bset->lfs_en_sect_bmarks ?
+                     fcb_log->fl_fcb.f_sector_cnt : 0);
+        if (bset->lfs_non_s_size >= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS)) {
+            bset->lfs_next_non_s = (bset->lfs_next_non_s + 1) %
+                                   MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS);
+        } else {
+            if (!bset->lfs_non_s_size) {
+                /* First non-sector bmark position */
+                bset->lfs_next_non_s = pos;
+            }

Review Comment:
   What about:
   ```
   323             if (!bset->lfs_non_s_size) {
   324                 /* First non-sector bmark position */
   325                 bset->lfs_next_non_s = pos;
   326             }
   ```
   



-- 
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: notifications-unsubscr...@mynewt.apache.org

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

Reply via email to