Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hw/mcu/native/src/hal_flash.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ static int
flash_native_write_internal(uint32_t address, const void *src, uint32_t length,
int allow_overwrite)
{
static uint8_t buf[256];
uint8_t buf[256] = { 0 };
uint32_t cur;
uint32_t end;
int chunk_sz;
Expand Down
208 changes: 206 additions & 2 deletions sys/log/full/include/log/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,46 @@ struct log_storage_info {
};
#endif

#if MYNEWT_VAL(LOG_FLAGS_TRAILER)

#define LOG_TRAILER_LEN_SIZE 2

/** @typedef log_trailer_free_func_t
* @brief Callback that frees the trailer buffer, to be called by user of the
* library once append returns, either successfully or unsuccessfully
*
* @param log The log the trailer is being appended to
* @param om mbuf pointer to be freed
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success, non-zero on failure
*/
typedef int log_trailer_free_func_t(struct log *log, struct os_mbuf *om, void *arg);

/** @typedef log_trailer_get_data_func_t
* @brief Callback that is executed each time the corresponding log entry is
* appended to
*
* @param log The log the trailer needs to be appended to
* @param om mbuf pointer to be set to mbuf which contains
* trailer data
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success, non-zero on failure
*/
typedef int log_trailer_get_data_func_t(struct log *log, struct os_mbuf **om, void *arg);

/** @typedef log_trailer_reset_data_func_t
* @brief Callback used to reset trailer data per log
*
* @param log The log the trailer is to be read from
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success, non-zero on failure.
*/
typedef int log_trailer_reset_data_func_t(struct log *log, void *arg);
#endif

typedef int (*log_walk_func_t)(struct log *, struct log_offset *log_offset,
const void *dptr, uint16_t len);

Expand All @@ -99,6 +139,7 @@ typedef int (*lh_append_mbuf_body_func_t)(struct log *log,
typedef int (*lh_walk_func_t)(struct log *,
log_walk_func_t walk_func, struct log_offset *log_offset);
typedef int (*lh_flush_func_t)(struct log *);
typedef uint16_t (*lh_read_entry_len_func_t)(struct log *log, const void *dptr);
#if MYNEWT_VAL(LOG_STORAGE_INFO)
typedef int (*lh_storage_info_func_t)(struct log *, struct log_storage_info *);
#endif
Expand All @@ -118,6 +159,7 @@ struct log_handler {
lh_walk_func_t log_walk;
lh_walk_func_t log_walk_sector;
lh_flush_func_t log_flush;
lh_read_entry_len_func_t log_read_entry_len;
#if MYNEWT_VAL(LOG_STORAGE_INFO)
lh_storage_info_func_t log_storage_info;
#endif
Expand All @@ -131,8 +173,9 @@ struct log_handler {
/* Image hash length to be looged */
#define LOG_IMG_HASHLEN 4

/* Flags used to indicate type of data in reserved payload*/
/* Flags used to indicate type of data in reserved payload */
#define LOG_FLAGS_IMG_HASH (1 << 0)
#define LOG_FLAGS_TRAILER (1 << 1)

#if MYNEWT_VAL(LOG_VERSION) == 3
struct log_entry_hdr {
Expand Down Expand Up @@ -204,6 +247,20 @@ STATS_SECT_END
#define LOG_STATS_INCN(log, name, cnt)
#endif

#if MYNEWT_VAL(LOG_FLAGS_TRAILER)
/* Trailer support callbacks */
struct log_trailer_handler {
/* Frees the trailer buffer */
log_trailer_free_func_t *lth_trailer_free;
/* Trailer append callback used to append trailer to the log entry */
log_trailer_get_data_func_t *lth_trailer_get_data;
/* Trailer reset data callback used to reset the trailer data
* per log which is defined by the application registering these callbacks
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in some other comment, there's no "user data" in the callback registration API.

log_trailer_reset_data_func_t *lth_trailer_reset_data;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anyone can figure out how to use these callbacks.

What is the difference between trailer_len and trailer_data_len? The latter doesn't seem to be used anywhere.
What is the purpose of trailer_reset_data? What is trailer data and how it's managed?
How the application should handle append callbacks?

and so on...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • So, earlier I had l_trailer_arg which you wanted me to make private so, I moved that out. Now, there needs to be a way to reset that trailer data which is what trailer_reset_data allows. If you have any other way to perform a reset, please let me know. This is called from log_flush.
  • trailer_len helps user of the library to get the length of the trailer as the name suggest.
  • trailer_data_len helps user of the library get the length of the trailer data. This was used earlier before I got rid of l_trailer_arg but now since the callbacks take care of the trailer data. I can remove it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you have in mind in this context is the "user data" that was supposed to be provided when registering callbacks. In that context, some kind of flush callback probably would make sense, but there's no "user data" in this PR so not sure what you want to reset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we do need to flush the data. So, do you have an alternate approach in mind ?

#endif

struct log {
const char *l_name;
const struct log_handler *l_log;
Expand All @@ -219,6 +276,23 @@ struct log {
#if MYNEWT_VAL(LOG_STATS)
STATS_SECT_DECL(logs) l_stats;
#endif
#if MYNEWT_VAL(LOG_INIT_CB)
/* Custom log init callback to be called by the last hdr
* read function to read custom data from log entries
* at init
*/
log_walk_func_t l_init_cb;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this also a syscfg and disabled by default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

#endif

#if MYNEWT_VAL(LOG_FLAGS_TRAILER)
/* Log trailer support */
/* Void argument for callbacks registeration */
void *l_tr_arg;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this argument shall be passed to all callbacks, otherwise it doesn't make any sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it is part of the log structure. So, all callbacks already have access to it. I can add it if you want but its just two copies of the same pointer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole point of user data for callbacks is that it's passed as an argument to each callback. application code (e.g. callbacks) should not access members of log structure directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

/* Trailer mbuf */
struct os_mbuf *l_tr_om;
/* Trailer handler with callbacks */
struct log_trailer_handler *l_th;
#endif
};

/* Log system level functions (for all logs.) */
Expand Down Expand Up @@ -514,6 +588,121 @@ void log_printf(struct log *log, uint8_t module, uint8_t level,
int log_read(struct log *log, const void *dptr, void *buf, uint16_t off,
uint16_t len);

#if MYNEWT_VAL(LOG_INIT_CB)
/**
* Reads entry length from the specified log.
*
* @return The number of bytes of entry length; 0 on failure.
*/
uint16_t
log_read_entry_len(struct log *log, const void *dptr);

#if MYNEWT_VAL(LOG_FLAGS_TRAILER)

/**
* @brief Read log trailer of a specific entry
*
* @param log The log to read trailer data from
* @param dptr Data pointer to the log entry
* @param buf Pointer to the buffer to read the trailer into
*
* @return 0 on success; nonzero on failure.
*/
uint16_t
log_read_trailer(struct log *log, const void *dptr, uint8_t *buf);

/**Add commentMore actions
* @brief Returns trailer data in an mbuf specified by the caller
*
* @param log The log to append to
* @param om mbuf pointer to be set to buffer which contains
* trailer data
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success; nonzero on failure.
*/
static inline int
log_trailer_get_data(struct log *log, struct os_mbuf **om, void *arg)
{
if (log->l_th && log->l_th->lth_trailer_get_data) {
return log->l_th->lth_trailer_get_data(log, om, arg);
}
return SYS_ENOTSUP;
}

/**
* @brief Callback that frees the trailer buffer, to be called by user of the library
* once append returns, either successfully or unsuccessfully
*
* @param log The log the trailer is being appended to
* @param om mbuf pointer to be freed
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success; nonzero on failure.
*/
static inline int
log_trailer_free(struct log *log, struct os_mbuf *om, void *arg)
{
if (log->l_th && log->l_th->lth_trailer_free) {
return log->l_th->lth_trailer_free(log, om, arg);
}
return SYS_ENOTSUP;
}

/**
* @brief Resets the trailer data in the log, calling user callback
*
* @param log The log to reset.
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success; nonzero on failure.
*/
static inline int
log_trailer_reset_data(struct log *log, void *arg)
{
if (log->l_th && log->l_th->lth_trailer_reset_data) {
return log->l_th->lth_trailer_reset_data(log, arg);
}
return SYS_ENOTSUP;
}

/**
* @brief Register trailer callbacks
*
* @param log The log to read from.
* @param lth The trailer handler to register.
* @param arg Argument to be passed to the trailer handler
*
* @return 0 on success; nonzero on failure.
*/
static inline void
log_trailer_cbs_register(struct log *log, struct log_trailer_handler *lth,
void *arg)
{
if (!lth || !log) {
return;
}

log->l_th = lth;
log->l_tr_arg = arg;
}
#endif

/**
* @brief Register Log init callback to be called by log_read_last_hdr
* while reading the last log entry at init
*
* @param log The log to register.
* @param cb The callback to associate with the log.
*
*/
static inline void
log_register_init_cb(struct log *log, log_walk_func_t cb)
{
log->l_init_cb = cb;
}
#endif

/**
* @brief Reads a single log entry header.
*
Expand All @@ -531,7 +720,7 @@ int log_read_hdr(struct log *log, const void *dptr, struct log_entry_hdr *hdr);
* @brief Reads the header length
*
* @param hdr Ptr to the header
*
*
* @return Length of the header
*/
uint16_t
Expand Down Expand Up @@ -738,6 +927,21 @@ int log_set_watermark(struct log *log, uint32_t index);
int
log_fill_current_img_hash(struct log_entry_hdr *hdr);

/**
* @brief Reads the final log entry's header and processes trailer if the flag
* indicates so from the specified log only if the log supports trailers
* and mynewt val LOG_FLAGS_TRAILER is 1
*
* @param log The log to read from.
* @param out_hdr On success, the last entry header gets written
* here.
* @param trailer_exists Pointer to a boolean
*
* @return 0 on success; nonzero on failure.
*/
int log_read_last_hdr_trailer(struct log *log, struct log_entry_hdr *out_hdr,
bool *trailer_exists);

/* Handler exports */
#if MYNEWT_VAL(LOG_CONSOLE)
extern const struct log_handler log_console_handler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ TEST_CASE_SELF(log_test_case_2logs)
{
int rc;
struct fcb_log fcb_log1;
struct log log1;
struct log log1 = {0};
struct fcb_log fcb_log2;
struct log log2;
struct log log2 = {0};

ltu_setup_2fcbs(&fcb_log1, &log1, &fcb_log2, &log2);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ ltcwc_append_cb(struct log *log, uint32_t idx)
TEST_CASE_SELF(log_test_case_append_cb)
{
struct fcb_log fcb_log;
struct log log;
struct log log = {0};

ltu_setup_fcb(&fcb_log, &log);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TEST_CASE_SELF(log_test_case_cbmem_append_body)
{
struct cbmem cbmem;
struct log log;
struct log log = {0};
char *str;
int i;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ TEST_CASE_SELF(log_test_case_cbmem_append_mbuf)
{
struct cbmem cbmem;
struct os_mbuf *om;
struct log log;
char *str;
struct log log = {0};
int rc;
int i;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_CASE_SELF(log_test_case_cbmem_append_mbuf_body)
{
struct cbmem cbmem;
struct os_mbuf *om;
struct log log;
struct log log = {0};
char *str;
int rc;
int i;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TEST_CASE_SELF(log_test_case_cbmem_printf)
{
struct cbmem cbmem;
struct log log;
struct log log = {0};
char *str;
int i;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TEST_CASE_SELF(log_test_case_fcb_append)
{
struct fcb_log fcb_log;
struct log log;
struct log log = {0};
uint8_t buf[256];
char *str;
int body_len;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TEST_CASE_SELF(log_test_case_fcb_append_body)
{
struct fcb_log fcb_log;
struct log log;
struct log log = {0};
char *str;
int i;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_CASE_SELF(log_test_case_fcb_append_mbuf)
{
struct fcb_log fcb_log;
struct os_mbuf *om;
struct log log;
struct log log = {0};
char *str;
int rc;
int i;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_CASE_SELF(log_test_case_fcb_append_mbuf_body)
{
struct fcb_log fcb_log;
struct os_mbuf *om;
struct log log;
struct log log = {0};
char *str;
int rc;
int i;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TEST_CASE_SELF(log_test_case_fcb_printf)
{
struct fcb_log fcb_log;
struct log log;
struct log log = {0};
char *str;
int i;

Expand Down
Loading
Loading