kasjer commented on code in PR #3413:
URL: https://github.com/apache/mynewt-core/pull/3413#discussion_r2048347464


##########
sys/log/full/src/log_fcb.c:
##########
@@ -914,6 +914,8 @@ log_fcb_copy_entry(struct log *log, struct fcb_entry *entry,
     uint16_t hdr_len;
     int dlen;
     int rc;
+    struct fcb_log fcb_log_tmp = {0};

Review Comment:
   Initialization is not needed whole buffer will be overwritten later



##########
sys/log/full/src/log_fcb.c:
##########
@@ -932,12 +934,22 @@ log_fcb_copy_entry(struct log *log, struct fcb_entry 
*entry,
         goto err;
     }
 
-    /* Changing the fcb to be logged to be dst fcb */
+    /* Cache fcb_log pointer */
     fcb_tmp = &((struct fcb_log *)log->l_arg)->fl_fcb;
+    fcb_log_ptr = (struct fcb_log *)log->l_arg;
 
-    log->l_arg = dst_fcb;
+    /* Cache the fcb log, so that we preserve original fcb pointer and
+     * bookmark settings
+     */
+    memcpy(&fcb_log_tmp, log->l_arg, sizeof(struct fcb_log));
+    fcb_log_tmp.fl_fcb = *dst_fcb;
+    log->l_arg = &fcb_log_tmp;
     rc = log_fcb_append(log, data, dlen);
-    log->l_arg = fcb_tmp;
+
+    /* Restore the original fcb_log pointer */
+    log->l_arg = fcb_log_ptr;
+    ((struct fcb_log *)log->l_arg)->fl_fcb = *fcb_tmp;

Review Comment:
   In current form `fcb_tmp` is not used for anything compiler will detect that 
this line is equivalent to
   ```c
     *fcb_tmp = *fcb_tmp;
   ```
   and will not generate code for assignment.



-- 
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]

Reply via email to