vrahane commented on code in PR #3365:
URL: https://github.com/apache/mynewt-core/pull/3365#discussion_r1981985723
##########
sys/log/full/src/log_fcb_bmark.c:
##########
@@ -61,74 +195,256 @@ log_fcb_rotate_bmarks(struct fcb_log *fcb_log)
i--;
}
bset->lfs_size--;
- bset->lfs_next = bset->lfs_size;
}
}
void
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_sect = 0;
+ fcb_log->fl_bset.lfs_non_sect_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) {
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+ if (i < fcb_log->fl_bset.lfs_sect_cap) {
+ /* Jump to the non-sector bookmarks since sector
+ * bookmarks are empty here on
+ */
+ i = fcb_log->fl_bset.lfs_sect_cap - 1;
+ continue;
+ }
+#else
+ /* Empty non-sector bookmark, nothing more to do
+ * Previous closest bookmark is the closest one */
+ break;
+#endif
+ }
+#elif MYNEWT_VAL(LOG_FCB2)
+ if (!fcb_log->fl_bset.lfs_bmarks[i].lfb_entry.fe_range) {
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+ if (i < fcb_log->fl_bset.lfs_sect_cap) {
+ /* Jump to the non-sector bookmarks since sector
+ * bookmarks are empty here on
+ */
+ i = fcb_log->fl_bset.lfs_sect_cap - 1;
+ continue;
+ }
+#else
+ /* Empty non-sector bookmark, nothing more to do
+ * Previous closest bookmark is the closest one */
+ break;
+#endif
+ }
+#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_DEBUG(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 int
+log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
+ uint32_t index)
#elif MYNEWT_VAL(LOG_FCB2)
-void
-log_fcb_add_bmark(struct fcb_log *fcb_log, const struct fcb2_entry *entry,
- uint32_t index)
+static int
+log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
+ uint32_t index)
#endif
{
struct log_fcb_bset *bset;
bset = &fcb_log->fl_bset;
- if (bset->lfs_cap == 0) {
- return;
+ bset->lfs_bmarks[bset->lfs_next_sect] = (struct log_fcb_bmark) {
+ .lfb_entry = *entry,
+ .lfb_index = index,
+ };
+
+ if (bset->lfs_size < fcb_log->fl_bset.lfs_sect_cap) {
+ bset->lfs_size++;
+ bset->lfs_next_sect = (bset->lfs_next_sect - 1) %
+ fcb_log->fl_bset.lfs_sect_cap;
Review Comment:
`log_fcb_init_sector_bmarks()` adds the bookmarks. It initializes
`bset->lfs_next_sect = bset->lfs_sect_cap - 1;` Check line `log_fcb_bmark.c:
58`. `mod` helps `bset->lfs_next_sect` decrement correctly. If/Else is not
required. Edge cases explained below:
Consider `fcb_log->fl_bset.lfs_sect_cap` is set to `5`.
### Edge Case 1: Initial Value at Maximum
If `bset->lfs_next_sect` starts at the maximum value of
`fcb_log->fl_bset.lfs_sect_cap - 1` (which is 4), the sequence of values will
be:
- Initial value: 4
- After first iteration: (4 - 1) % 5 = 3
- After second iteration: (3 - 1) % 5 = 2
- After third iteration: (2 - 1) % 5 = 1
- After fourth iteration: (1 - 1) % 5 = 0
- After fifth iteration: (0 - 1) % 5 = 4 (since -1 % 5 = 4 in unsigned
arithmetic)
### Edge Case 2: Initial Value at Zero
If `bset->lfs_next_sect` starts at 0, the sequence of values will be:
- Initial value: 0
- After first iteration: (0 - 1) % 5 = 4 (since -1 % 5 = 4 in unsigned
arithmetic)
- After second iteration: (4 - 1) % 5 = 3
- After third iteration: (3 - 1) % 5 = 2
- After fourth iteration: (2 - 1) % 5 = 1
- After fifth iteration: (1 - 1) % 5 = 0
### Edge Case 3: Large Initial Value
If `bset->lfs_next_sect` starts at a large value, say 10, the sequence of
values will be:
- Initial value: 10
- After first iteration: (10 - 1) % 5 = 9 % 5 = 4
- After second iteration: (4 - 1) % 5 = 3
- After third iteration: (3 - 1) % 5 = 2
- After fourth iteration: (2 - 1) % 5 = 1
- After fifth iteration: (1 - 1) % 5 = 0
### Edge Case 4: Negative Initial Value
Since `bset->lfs_next_sect` is a `uint32_t`, it cannot be negative. However,
if we consider the behavior of the modulo operation with negative numbers in
general, it would wrap around to a positive value within the range of
`fcb_log->fl_bset.lfs_sect_cap`.
--
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]
