Skip to content

Commit 84c36e6

Browse files
committed
sys/log: Fix trailer length offset in log_fcb
- Trailer buffer pointer, data and callbacks should not be zeroed out on registeration. This is because log_registeration does not just initialize the fields, it also reads the most recent log entry to update the index and also call a custom init callback which can be reading the trailer. - The trailer_alignment of 0 was making the offset for the trailer_len to be wrong. Now, we do not try to copy padding for the trailer_alignment if it is 0. This change along with the removal of zeroing out the trailer info in the log structure was causing the tests to fail since the offset was wrong. Correcting the trailer_len offset fixes everything. - Make log_trailer_cbs_register experimental - This is to allow log registeration for trailers in a combined fashion
1 parent 12fd9e7 commit 84c36e6

File tree

3 files changed

+18
-14
lines changed

3 files changed

+18
-14
lines changed

sys/log/full/include/log/log.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,10 @@ log_trailer_reset_data(struct log *log, void *arg)
667667
}
668668

669669
/**
670-
* @brief Register trailer callbacks
670+
* @brief Register trailer callbacks, this is an experimental API
671+
* and may be changed/deprecated later. This needs to be
672+
* called before log_register() if the trailer data needs
673+
* to be initialized when the log gets registered.
671674
*
672675
* @param log The log to read from.
673676
* @param lth The trailer handler to register.

sys/log/full/src/log.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@ log_read_last_hdr(struct log *log, struct log_entry_hdr *out_hdr)
437437

438438
/*
439439
* Associate an instantiation of a log with the logging infrastructure
440+
* If trailer data needs to be read, the trailer callbacks should be
441+
* registered before calling this function. They are set using
442+
* log_trailer_cbs_register().
440443
*/
441444
int
442445
log_register(const char *name, struct log *log, const struct log_handler *lh,
@@ -462,12 +465,6 @@ log_register(const char *name, struct log *log, const struct log_handler *lh,
462465
log->l_idx = 0;
463466
#endif
464467

465-
#if MYNEWT_VAL(LOG_FLAGS_TRAILER)
466-
log->l_tr_om = NULL;
467-
log->l_tr_arg = NULL;
468-
log->l_th = NULL;
469-
#endif
470-
471468
if (!log_registered(log)) {
472469
#if MYNEWT_VAL(LOG_STATS)
473470
stats_init(STATS_HDR(log->l_stats),

sys/log/full/src/log_fcb.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ log_fcb_append_mbuf_body(struct log *log, const struct log_entry_hdr *hdr,
542542
struct fcb_log *fcb_log;
543543
int len;
544544
int rc;
545+
uint16_t buflen = 0;
545546
#if MYNEWT_VAL(LOG_FLAGS_TRAILER)
546547
int trailer_alignment = 0;
547548
uint8_t pad[LOG_FCB_MAX_ALIGN] = {0};
@@ -558,7 +559,8 @@ log_fcb_append_mbuf_body(struct log *log, const struct log_entry_hdr *hdr,
558559
return SYS_ENOTSUP;
559560
}
560561

561-
len = log_hdr_len(hdr) + os_mbuf_len(om);
562+
buflen = os_mbuf_len(om);
563+
len = log_hdr_len(hdr) + buflen;
562564

563565
#if MYNEWT_VAL(LOG_FLAGS_TRAILER)
564566
if (hdr->ue_flags & LOG_FLAGS_TRAILER) {
@@ -616,10 +618,12 @@ log_fcb_append_mbuf_body(struct log *log, const struct log_entry_hdr *hdr,
616618
*/
617619

618620
if (log->l_tr_om) {
619-
/* Copy mbuf chain len */
620-
rc = os_mbuf_copyinto(om, trailer_len, pad, trailer_alignment);
621-
if (rc) {
622-
return rc;
621+
if (trailer_alignment) {
622+
/* Copy padding for trailer alignment */
623+
rc = os_mbuf_copyinto(om, buflen, pad, trailer_alignment);
624+
if (rc) {
625+
return rc;
626+
}
623627
}
624628

625629
/* Append from the trailer */
@@ -630,8 +634,8 @@ log_fcb_append_mbuf_body(struct log *log, const struct log_entry_hdr *hdr,
630634

631635
om = os_mbuf_pullup(om, LOG_TRAILER_LEN_SIZE);
632636
/* Copy the trailer length */
633-
rc = os_mbuf_copyinto(om, trailer_len, &trailer_len,
634-
LOG_TRAILER_LEN_SIZE);
637+
rc = os_mbuf_copyinto(om, buflen + trailer_alignment + trailer_len,
638+
&trailer_len, LOG_TRAILER_LEN_SIZE);
635639
if (rc) {
636640
return rc;
637641
}

0 commit comments

Comments
 (0)