sjanc commented on code in PR #3382:
URL: https://github.com/apache/mynewt-core/pull/3382#discussion_r1989867550
##########
fs/fcb/src/fcb.c:
##########
@@ -118,14 +124,29 @@ fcb_free_sector_cnt(struct fcb *fcb)
break;
}
}
+
+ os_mutex_release(&fcb->f_mtx);
+
return i;
}
int
fcb_is_empty(struct fcb *fcb)
{
- return (fcb->f_active.fe_area == fcb->f_oldest &&
+ int rc = 0;
+ bool ret = false;
+
+ rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
+ if (rc && rc != OS_NOT_STARTED) {
+ return FCB_ERR_ARGS;
Review Comment:
Would be good to note in header file that negative value means error and not
that it is not empty (non-zero value)
(alternative would be to make this int fcb_is_empty(struct fcb *fcb, bool
*empty) for more explicit error vs return value
##########
fs/fcb/src/fcb.c:
##########
@@ -257,14 +286,21 @@ fcb_offset_last_n(struct fcb *fcb, uint8_t entries,
int
fcb_clear(struct fcb *fcb)
{
- int rc;
+ int rc = 0;
+
+ rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
+ if (rc && rc != OS_NOT_STARTED) {
+ return FCB_ERR_ARGS;
+ }
Review Comment:
loop below could use _nolock() variants to avoid nested locking
##########
fs/fcb/src/fcb.c:
##########
@@ -226,8 +247,14 @@ fcb_offset_last_n(struct fcb *fcb, uint8_t entries,
struct fcb_entry *last_n_entry)
{
struct fcb_entry loc;
+ int rc = 0;
int i;
+ rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
+ if (rc && rc != OS_NOT_STARTED) {
+ return FCB_ERR_ARGS;
+ }
+
Review Comment:
there is fcb_getnext_nolock() which could be used in loop below to avoid
nested locking
##########
fs/fcb/src/fcb.c:
##########
@@ -110,6 +110,12 @@ fcb_free_sector_cnt(struct fcb *fcb)
{
int i;
struct flash_area *fa;
+ int rc = 0;
+
+ rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
+ if (rc && rc != OS_NOT_STARTED) {
+ return FCB_ERR_ARGS;
Review Comment:
Would be good to note in header file that negative value means error (or
make cnt output parameter similar to suggestion in fcb_is_empty()) (either is
fine for me)
--
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
