vrahane commented on code in PR #3365:
URL: https://github.com/apache/mynewt-core/pull/3365#discussion_r1956978521
##########
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;
+ }
+ }
Review Comment:
What do you mean ? pos tracks i. If the bookmark we are adding has a higher
index than the index in the bookmark at i, pos will become that value. If index
is <= index of bookmark at i, we keep incrementing pos and iterating forward
until we find the right i in which case we break and come out of the loop. How
is setting pos to `bset->lfs_size` even correct ? If you try it out, you will
see it is correct the way it is. I have actually written the sorting code
separately and then added here after testing it. If you feel doubtful can you
please try it out ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
