From b9c48d9f4e69bb03049a65cdbe380b341a3e247e Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Wed, 2 Mar 2016 10:48:28 -0800 Subject: [PATCH 01/11] Initial sysparam implementation Problems with inability to write individual bytes to flash. Need to reorganize to read/write word-multiples instead. --- core/include/esp/types.h | 1 + core/include/sysparam.h | 42 +++ core/sysparam.c | 616 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 659 insertions(+) create mode 100644 core/include/sysparam.h create mode 100644 core/sysparam.c diff --git a/core/include/esp/types.h b/core/include/esp/types.h index cb816da..53c4cd4 100644 --- a/core/include/esp/types.h +++ b/core/include/esp/types.h @@ -3,6 +3,7 @@ #include #include +#include typedef volatile uint32_t *esp_reg_t; diff --git a/core/include/sysparam.h b/core/include/sysparam.h new file mode 100644 index 0000000..08bf506 --- /dev/null +++ b/core/include/sysparam.h @@ -0,0 +1,42 @@ +#ifndef _SYSPARAM_H_ +#define _SYSPARAM_H_ + +#include + +#ifndef SYSPARAM_REGION_SECTORS +// Number of (4K) sectors that make up a sysparam region. Total sysparam data +// cannot be larger than this. Note that the actual amount of used flash +// space will be *twice* this amount. +#define SYSPARAM_REGION_SECTORS 1 +#endif + +typedef struct { + char *key; + uint8_t *value; + size_t key_len; + size_t value_len; + size_t bufsize; + struct sysparam_context *ctx; +} sysparam_iter_t; + +typedef enum { + SYSPARAM_ERR_NOMEM = -5, + SYSPARAM_ERR_BADDATA = -4, + SYSPARAM_ERR_IO = -3, + SYSPARAM_ERR_FULL = -2, + SYSPARAM_ERR_BADARGS = -1, + SYSPARAM_OK = 0, + SYSPARAM_NOTFOUND = 1, +} sysparam_status_t; + +sysparam_status_t sysparam_init(uint32_t base_addr, bool create); +sysparam_status_t sysparam_get_raw(const char *key, uint8_t **destptr, size_t *actual_length); +sysparam_status_t sysparam_get_string(const char *key, char **destptr); +sysparam_status_t sysparam_get_raw_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); +sysparam_status_t sysparam_put_raw(const char *key, const uint8_t *value, size_t value_len); +sysparam_status_t sysparam_put_string(const char *key, const char *value); +sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter); +sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter); +void sysparam_iter_end(sysparam_iter_t *iter); + +#endif /* _SYSPARAM_H_ */ diff --git a/core/sysparam.c b/core/sysparam.c new file mode 100644 index 0000000..65bfda5 --- /dev/null +++ b/core/sysparam.c @@ -0,0 +1,616 @@ +#include +#include +#include +#include + +//TODO: make this properly threadsafe + +#define SYSPARAM_MAGIC 0x70524f45 // "EORp" in little-endian +#define SYSPARAM_REGION_HEADER_SIZE 4 +#define ENTRY_OVERHEAD 4 +#define DEFAULT_ITER_BUF_SIZE 64 +#define MAX_KEY_ID 0x7e + +typedef struct sysparam_context { + uint32_t key_addr; + uint32_t value_addr; + uint32_t end_addr; + size_t key_len; + size_t value_len; + size_t compactable; + uint8_t key_id; + uint8_t max_key_id; + bool length_error; +} sysparam_context_t; + +//#define CHECK_FLASH_OP(x) if ((x) != SPI_FLASH_RESULT_OK) return SYSPARAM_ERR_IO; +#include +#define CHECK_FLASH_OP(x) do { int __x = (x); if ((__x) != SPI_FLASH_RESULT_OK) { printf("FLASH ERR: %s: %d\n", #x, __x); return SYSPARAM_ERR_IO; } } while (0); + +/* Internal data structures */ + +static struct { + uint32_t cur_addr; + uint32_t alt_addr; + size_t region_size; +} sysparam_info; + +/* Internal routines */ + +#define max(x, y) ((x) > (y) ? (x) : (y)) +#define min(x, y) ((x) < (y) ? (x) : (y)) + +static sysparam_status_t format_region(uint32_t addr) { + uint16_t sector = addr / sdk_flashchip.sector_size; + int i; + + for (i = 0; i < SYSPARAM_REGION_SECTORS; i++) { + CHECK_FLASH_OP(sdk_spi_flash_erase_sector(sector + i)); + } + return SYSPARAM_OK; +} + +static sysparam_status_t write_region_header(uint32_t addr) { + uint32_t magic = SYSPARAM_MAGIC; + CHECK_FLASH_OP(sdk_spi_flash_write(addr, &magic, 4)); + return SYSPARAM_OK; +} + +static void init_context(sysparam_context_t *ctx) { + memset(ctx, 0, sizeof(*ctx)); + ctx->key_addr = sysparam_info.cur_addr + SYSPARAM_REGION_HEADER_SIZE; +} + +// Scan through the region to find the location of the key entry matching the +// specified name. +static sysparam_status_t find_key(sysparam_context_t *ctx, const char *key, size_t key_len, uint8_t *buffer) { + size_t payload_len; + uint8_t entry_id; + + // Are we already at the end? + if (ctx->key_addr == ctx->end_addr) return SYSPARAM_NOTFOUND; + + while (ctx->key_addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { + if (ctx->length_error) { + // Our last entry's length fields didn't match, which means we have + // no idea whether we're in the right place anymore. Can't + // continue. + //FIXME: print an error + return SYSPARAM_ERR_BADDATA; + } + if (ctx->key_len) { + ctx->key_addr += ctx->key_len + ENTRY_OVERHEAD; + } + + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr, buffer, 2)); + payload_len = buffer[0]; + entry_id = buffer[1]; + ctx->key_len = payload_len; + + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + payload_len + 2, buffer, 2)); + // checksum = buffer[0]; //FIXME + if (buffer[1] != payload_len) { + ctx->length_error = true; + } + + if (entry_id == 0xff) { + // End of entries + break; + } else if (!entry_id) { + // Deleted entry + } else if (!(entry_id & 0x80)) { + // Key definition + ctx->max_key_id = max(ctx->max_key_id, entry_id); + if (!key) { + ctx->key_id = entry_id; + return SYSPARAM_OK; + } + if (payload_len == key_len) { + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + 2, buffer, key_len)); + //FIXME: check checksum + // If checksum checks out, clear length_error + if (!memcmp(key, buffer, key_len)) { + ctx->key_id = entry_id; + return SYSPARAM_OK; + } + } + } + } + ctx->end_addr = ctx->key_addr; + ctx->key_len = 0; + return SYSPARAM_NOTFOUND; +} + +// Scan through the region to find the location of the value entry matching the +// key found by `find_key` +static sysparam_status_t find_value(sysparam_context_t *ctx) { + uint32_t addr = ctx->key_addr; + size_t payload_len = ctx->key_len; + bool length_error = ctx->length_error; + uint8_t value_id = ctx->key_id | 0x80; + uint8_t entry_id; + uint8_t buffer[2]; + + while (addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { + if (length_error) { + // Our last entry's length fields didn't match, which means we have + // no idea whether we're in the right place anymore. Can't + // continue. + //FIXME: print an error + return SYSPARAM_ERR_BADDATA; + } + addr += payload_len + ENTRY_OVERHEAD; + + CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, 2)); + payload_len = buffer[0]; + entry_id = buffer[1]; + + CHECK_FLASH_OP(sdk_spi_flash_read(addr + payload_len + 2, buffer, 2)); + //checksum = buffer[0]; //FIXME + if (buffer[1] != payload_len) { + length_error = true; + } + + if (entry_id == 0xff) { + // End of entries + break; + } else if (!entry_id) { + // Deleted entry + } else if (!(entry_id & 0x80)) { + // Key entry. Make sure we update max_key_id. + ctx->max_key_id = max(ctx->max_key_id, entry_id); + } else if (entry_id == value_id) { + // We found our value + ctx->value_addr = addr; + ctx->value_len = payload_len; + return SYSPARAM_OK; + } + } + ctx->end_addr = addr; + ctx->value_len = 0; + return SYSPARAM_NOTFOUND; +} + +// Scan through any remaining data in the region until we get to the end, +// updating ctx as we go. +// NOTE: This assumes you've already called `find_key`/`find_value` (if you +// care about those things). It only looks for the end. +static sysparam_status_t find_end(sysparam_context_t *ctx) { + uint32_t addr; + size_t payload_len; + bool length_error = ctx->length_error; + uint8_t entry_id; + uint8_t buffer[2]; + + if (ctx->end_addr) { + return SYSPARAM_OK; + } + if (ctx->value_addr) { + addr = ctx->value_addr; + payload_len = ctx->value_len; + } else { + addr = ctx->key_addr; + payload_len = ctx->key_len; + } + while (addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { + if (length_error) { + // Our last entry's length fields didn't match, which means we have + // no idea whether we're in the right place anymore. Can't + // continue. + //FIXME: print an error + return SYSPARAM_ERR_BADDATA; + } + addr += payload_len + ENTRY_OVERHEAD; + + CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, 2)); + payload_len = buffer[0]; + entry_id = buffer[1]; + + CHECK_FLASH_OP(sdk_spi_flash_read(addr + payload_len + 2, buffer, 2)); + //checksum = buffer[0]; //FIXME + if (buffer[1] != payload_len) { + length_error = true; + } + + if (entry_id == 0xff) { + // End of entries + break; + } else if (!entry_id) { + // Deleted entry + } else if (!(entry_id & 0x80)) { + // Key entry. Make sure we update max_key_id. + ctx->max_key_id = max(ctx->max_key_id, entry_id); + } + } + ctx->end_addr = addr; + ctx->value_len = 0; + return SYSPARAM_OK; +} + +static sysparam_status_t read_key(sysparam_context_t *ctx, char *buffer, size_t buffer_size) { + if (!ctx->key_len) { + return SYSPARAM_NOTFOUND; + } + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + 2, buffer, min(buffer_size, ctx->key_len))); + //FIXME: check checksum + return SYSPARAM_OK; +} + +static sysparam_status_t read_value(sysparam_context_t *ctx, uint8_t *buffer, size_t buffer_size) { + if (!ctx->value_len) { + return SYSPARAM_NOTFOUND; + } + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->value_addr + 2, buffer, min(buffer_size, ctx->value_len))); + //FIXME: check checksum + return SYSPARAM_OK; +} + +static inline sysparam_status_t write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, size_t payload_len) { + uint8_t buffer[2]; + + buffer[0] = payload_len; + buffer[1] = id; + CHECK_FLASH_OP(sdk_spi_flash_write(addr, buffer, 2)); + CHECK_FLASH_OP(sdk_spi_flash_write(addr + 2, payload, payload_len)); + buffer[0] = 0; //FIXME: calculate checksum + buffer[1] = payload_len; + CHECK_FLASH_OP(sdk_spi_flash_write(addr + 2 + payload_len, buffer, 2)); + + return SYSPARAM_OK; +} + +static sysparam_status_t compact_params(sysparam_context_t *ctx, bool delete_current_value) { + uint32_t new_base = sysparam_info.alt_addr; + sysparam_status_t status; + uint32_t addr = new_base + SYSPARAM_REGION_HEADER_SIZE; + uint8_t current_key_id = 0; + uint32_t zero = 0; + sysparam_iter_t iter; + + status = format_region(new_base); + if (status < 0) return status; + status = sysparam_iter_start(&iter); + if (status < 0) return status; + + while (true) { + status = sysparam_iter_next(&iter); + if (status < 0) break; + + current_key_id++; + + if (iter.ctx->key_addr == ctx->key_addr) { + ctx->key_addr = addr; + } + if (iter.ctx->key_id == ctx->key_id) { + ctx->key_id = current_key_id; + } + + // Write the key to the new region + status = write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len); + if (status < 0) break; + addr += iter.key_len + ENTRY_OVERHEAD; + + if (iter.ctx->value_addr == ctx->value_addr) { + if (delete_current_value) { + // Don't copy the old value, since we'll just be deleting it + // and writing a new one as soon as we return. + ctx->value_addr = 0; + ctx->value_len = 0; + continue; + } else { + ctx->value_addr = addr; + } + } + // Copy the value to the new region + status = write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len); + if (status < 0) break; + addr += iter.value_len + ENTRY_OVERHEAD; + } + sysparam_iter_end(&iter); + + // If we broke out with an error, return the error instead of continuing. + if (status < 0) return status; + + // Fix up all the remaining bits of ctx to have correct values for the new + // (compacted) region. + ctx->end_addr = addr; + ctx->compactable = 0; + ctx->max_key_id = current_key_id; + ctx->length_error = false; + + // Switch to officially using the new region. + status = write_region_header(new_base); + if (status < 0) return status; + sysparam_info.alt_addr = sysparam_info.cur_addr; + sysparam_info.cur_addr = new_base; + // Zero out the old header, so future calls to sysparam_init() will know + // that the new one is the correct region to be looking at. + CHECK_FLASH_OP(sdk_spi_flash_write(sysparam_info.alt_addr, &zero, 4)); + + return SYSPARAM_OK; +} + +/* Public Functions */ + +sysparam_status_t sysparam_init(uint32_t base_addr, bool create) { + sysparam_status_t status; + size_t region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; + uint32_t magic; + int i; + + sysparam_info.region_size = region_size; + for (i = 0; i < 2; i++) { + CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + (i * region_size), &magic, 4)); + if (magic == SYSPARAM_MAGIC) { + sysparam_info.cur_addr = base_addr + i * region_size; + sysparam_info.alt_addr = base_addr + (i ^ 1) * region_size; + return SYSPARAM_OK; + } + } + // We couldn't find one already present, so create a new one at the + // specified address (if `create` is set). + if (create) { + sysparam_info.cur_addr = base_addr; + sysparam_info.alt_addr = base_addr + region_size; + status = format_region(base_addr); + if (status != SYSPARAM_OK) return status; + return write_region_header(base_addr); + } else { + return SYSPARAM_NOTFOUND; + } +} + +sysparam_status_t sysparam_get_raw(const char *key, uint8_t **destptr, size_t *actual_length) { + sysparam_context_t ctx; + sysparam_status_t status; + size_t key_len = strlen(key); + uint8_t *buffer = malloc(key_len + 2); + + if (!buffer) return SYSPARAM_ERR_NOMEM; + do { + init_context(&ctx); + status = find_key(&ctx, key, key_len, buffer); + if (status != SYSPARAM_OK) break; + + status = find_value(&ctx); + if (status != SYSPARAM_OK) break; + + buffer = realloc(buffer, ctx.value_len + 1); + if (!buffer) { + return SYSPARAM_ERR_NOMEM; + } + status = read_value(&ctx, buffer, ctx.value_len); + if (status != SYSPARAM_OK) break; + + // Zero-terminate the result, just in case (doesn't hurt anything for + // non-string data, and can avoid nasty mistakes if the caller wants to + // interpret the result as a string). + buffer[ctx.value_len] = 0; + + *destptr = buffer; + if (actual_length) *actual_length = ctx.value_len; + return SYSPARAM_OK; + } while (false); + + free(buffer); + *destptr = NULL; + if (actual_length) *actual_length = 0; + return status; +} + +sysparam_status_t sysparam_get_string(const char *key, char **destptr) { + // `sysparam_get_raw` will zero-terminate the result as a matter of course, + // so no need to do that here. + return sysparam_get_raw(key, (uint8_t **)destptr, NULL); +} + +sysparam_status_t sysparam_get_raw_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length) { + sysparam_context_t ctx; + sysparam_status_t status = SYSPARAM_OK; + size_t key_len = strlen(key); + + // Supplied buffer must be at least as large as the key, or 2 bytes, + // whichever is larger. + if (buffer_size < max(key_len, 2)) return SYSPARAM_ERR_NOMEM; + + if (actual_length) *actual_length = 0; + + init_context(&ctx); + status = find_key(&ctx, key, key_len, buffer); + if (status != SYSPARAM_OK) return status; + status = find_value(&ctx); + if (status != SYSPARAM_OK) return status; + status = read_value(&ctx, buffer, buffer_size); + if (status != SYSPARAM_OK) return status; + + if (actual_length) *actual_length = ctx.value_len; + return SYSPARAM_OK; +} + +sysparam_status_t sysparam_put_raw(const char *key, const uint8_t *value, size_t value_len) { + sysparam_context_t ctx; + sysparam_status_t status = SYSPARAM_OK; + size_t key_len = strlen(key); + uint8_t *buffer; + size_t free_space; + size_t needed_space; + bool free_value = false; + + if (!key_len) return SYSPARAM_ERR_BADARGS; + if (value_len && (value & 0x3)) { + // The passed value isn't word-aligned. This will be a problem later + // when calling `sdk_spi_flash_write`, so make a word-aligned copy. + buffer = malloc(value_len); + if (!buffer) return SYSPARAM_ERR_NOMEM; + memcpy(buffer, value, value_len); + value = buffer; + free_value = true; + } + // Create a working buffer for `find_key` to use. + buffer = malloc(key_len); + if (!buffer) { + if (free_value) free(value); + return SYSPARAM_ERR_NOMEM; + } + + + do { + init_context(&ctx); + status = find_key(&ctx, key, key_len, buffer); + if (status == SYSPARAM_OK) { + // Key already exists, see if there's a current value. + status = find_value(&ctx); + } + if (status < 0) break; + + if (value_len) { + if (ctx.value_len == value_len) { + // Are we trying to write the same value that's already there? + if (value_len > key_len) { + buffer = realloc(buffer, value_len); + if (!buffer) return SYSPARAM_ERR_NOMEM; + } + status = read_value(&ctx, buffer, value_len); + if (status < 0) break; + if (!memcmp(buffer, value, value_len)) { + // Yup! No need to do anything further, just leave the + // current value as-is. + status = SYSPARAM_OK; + break; + } + } + + // Write the new/updated value + status = find_end(&ctx); + if (status < 0) break; + + // Since we will be deleting the old value (if any) make sure that + // the compactable count includes the space taken up by that entry + // too (even though it's not actually deleted yet) + if (ctx.value_addr) { + ctx.compactable += ctx.value_len + ENTRY_OVERHEAD; + } + + // Append new value to the end, but first make sure we have enough + // space. + free_space = sysparam_info.region_size - (ctx.end_addr - sysparam_info.cur_addr); + needed_space = ENTRY_OVERHEAD + value_len; + if (!ctx.key_id) { + // We did not find a previous key entry matching this key. We will + // need to add a key entry as well. + key_len = strlen(key); + needed_space += ENTRY_OVERHEAD + key_len; + } + if (needed_space > free_space) { + if (needed_space > free_space + ctx.compactable) { + // Nothing we can do here.. We're full. + // (at least full enough that compacting won't help us store + // this value) + //FIXME: debug message + status = SYSPARAM_ERR_FULL; + break; + } + // We should have enough space after we compact things. + status = compact_params(&ctx, true); + if (status < 0) break; + } + + if (!ctx.key_id) { + // Write key entry for new key + ctx.key_id = ctx.max_key_id + 1; + if (ctx.key_id > MAX_KEY_ID) { + status = SYSPARAM_ERR_FULL; + break; + } + status = write_entry(ctx.end_addr, ctx.key_id, (uint8_t *)key, key_len); + if (status < 0) break; + ctx.end_addr += key_len + ENTRY_OVERHEAD; + } + + // Write new value + status = write_entry(ctx.end_addr, ctx.key_id | 0x80, value, value_len); + if (status < 0) break; + } + + // Delete old value (if present) by setting it's id to 0x00 + if (ctx.value_addr) { + buffer[0] = 0; + if (sdk_spi_flash_write(ctx.value_addr + 1, buffer, 1) != SPI_FLASH_RESULT_OK) { + status = SYSPARAM_ERR_IO; + break; + } + } + } while (false); + + if (free_value) free(value); + free(buffer); + return status; +} + +sysparam_status_t sysparam_put_string(const char *key, const char *value) { + return sysparam_put_raw(key, (const uint8_t *)value, strlen(value)); +} + +sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { + iter->bufsize = DEFAULT_ITER_BUF_SIZE; + iter->key = malloc(iter->bufsize); + if (!iter->key) { + iter->bufsize = 0; + return SYSPARAM_ERR_NOMEM; + } + iter->key_len = 0; + iter->value_len = 0; + iter->ctx = malloc(sizeof(sysparam_context_t)); + if (!iter->ctx) { + free(iter->key); + iter->bufsize = 0; + return SYSPARAM_ERR_NOMEM; + } + init_context(iter->ctx); + + return SYSPARAM_OK; +} + +sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { + uint8_t buffer[2]; + sysparam_status_t status; + size_t required_len; + + while (true) { + status = find_key(iter->ctx, NULL, 0, buffer); + if (status != 0) return status; + status = find_value(iter->ctx); + if (status < 0) return status; + if (status == SYSPARAM_NOTFOUND) continue; + required_len = iter->ctx->key_len + 1 + iter->ctx->value_len + 1; + if (required_len < iter->bufsize) { + iter->key = realloc(iter->key, required_len); + if (!iter->key) { + iter->bufsize = 0; + return SYSPARAM_ERR_NOMEM; + } + iter->bufsize = required_len; + } + + status = read_key(iter->ctx, iter->key, iter->bufsize); + if (status < 0) return status; + // Null-terminate the key + iter->key[iter->ctx->key_len] = 0; + iter->key_len = iter->ctx->key_len; + + iter->value = (uint8_t *)iter->key + iter->ctx->key_len + 1; + status = read_value(iter->ctx, iter->value, iter->bufsize - iter->ctx->key_len - 1); + if (status < 0) return status; + // Null-terminate the value (just in case) + iter->value[iter->ctx->value_len] = 0; + iter->value_len = iter->ctx->value_len; + + return SYSPARAM_OK; + } +} + +void sysparam_iter_end(sysparam_iter_t *iter) { + if (iter->key) free(iter->key); + if (iter->ctx) free(iter->ctx); +} + From ae16299f4a031f5942b70acd742f6b04d80b36b3 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Mon, 7 Mar 2016 09:14:14 -0800 Subject: [PATCH 02/11] sysparam improvements Mostly done, a few minor cleanups left. --- core/include/sysparam.h | 37 +- core/sysparam.c | 894 +++++++++++++++++++++++----------------- 2 files changed, 538 insertions(+), 393 deletions(-) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index 08bf506..6f158ee 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -10,6 +10,18 @@ #define SYSPARAM_REGION_SECTORS 1 #endif +typedef enum { + SYSPARAM_ERR_NOMEM = -6, + SYSPARAM_ERR_CORRUPT = -5, + SYSPARAM_ERR_IO = -4, + SYSPARAM_ERR_FULL = -3, + SYSPARAM_ERR_BADVALUE = -2, + SYSPARAM_ERR_NOINIT = -1, + SYSPARAM_OK = 0, + SYSPARAM_NOTFOUND = 1, + SYSPARAM_PARSEFAILED = 2, +} sysparam_status_t; + typedef struct { char *key; uint8_t *value; @@ -19,22 +31,17 @@ typedef struct { struct sysparam_context *ctx; } sysparam_iter_t; -typedef enum { - SYSPARAM_ERR_NOMEM = -5, - SYSPARAM_ERR_BADDATA = -4, - SYSPARAM_ERR_IO = -3, - SYSPARAM_ERR_FULL = -2, - SYSPARAM_ERR_BADARGS = -1, - SYSPARAM_OK = 0, - SYSPARAM_NOTFOUND = 1, -} sysparam_status_t; - -sysparam_status_t sysparam_init(uint32_t base_addr, bool create); -sysparam_status_t sysparam_get_raw(const char *key, uint8_t **destptr, size_t *actual_length); +sysparam_status_t sysparam_init(uint32_t base_addr); +sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); +sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length); +sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); sysparam_status_t sysparam_get_string(const char *key, char **destptr); -sysparam_status_t sysparam_get_raw_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); -sysparam_status_t sysparam_put_raw(const char *key, const uint8_t *value, size_t value_len); -sysparam_status_t sysparam_put_string(const char *key, const char *value); +sysparam_status_t sysparam_get_int(const char *key, int32_t *result); +sysparam_status_t sysparam_get_bool(const char *key, bool *result); +sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len); +sysparam_status_t sysparam_set_string(const char *key, const char *value); +sysparam_status_t sysparam_set_int(const char *key, int32_t value); +sysparam_status_t sysparam_set_bool(const char *key, bool value); sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter); sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter); void sysparam_iter_end(sysparam_iter_t *iter); diff --git a/core/sysparam.c b/core/sysparam.c index 65bfda5..d6fd4b1 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -1,37 +1,54 @@ #include #include +#include #include #include //TODO: make this properly threadsafe +//FIXME: reduce stack usage +//TODO: make stderr work for debug output + +#ifndef SYSPARAM_DEBUG +#define SYSPARAM_DEBUG 0 +#endif + +#define debug(level, format, ...) if (SYSPARAM_DEBUG >= (level)) { printf("%s" format "\n", "sysparam: ", ## __VA_ARGS__); } #define SYSPARAM_MAGIC 0x70524f45 // "EORp" in little-endian -#define SYSPARAM_REGION_HEADER_SIZE 4 -#define ENTRY_OVERHEAD 4 +#define SYSPARAM_STALEMAGIC 0x40524f45 // "EOR@" in little-endian +#define REGION_HEADER_SIZE 4 // NOTE: Must be multiple of 4 +#define ENTRY_HEADER_SIZE 4 // NOTE: Must be multiple of 4 #define DEFAULT_ITER_BUF_SIZE 64 #define MAX_KEY_ID 0x7e +#define SCAN_BUFFER_SIZE 8 // words -typedef struct sysparam_context { - uint32_t key_addr; - uint32_t value_addr; - uint32_t end_addr; - size_t key_len; - size_t value_len; +#define ROUND_TO_WORD_BOUNDARY(x) (((x) + 3) & 0xfffffffc) +#define ENTRY_SIZE(payload_len) (ENTRY_HEADER_SIZE + ROUND_TO_WORD_BOUNDARY(payload_len)) + +#define CHECK_FLASH_OP(x) do { int __x = (x); if ((__x) != SPI_FLASH_RESULT_OK) { debug(1, "FLASH ERR: %d", __x); return SYSPARAM_ERR_IO; } } while (0); + + +struct entry_header { + uint8_t prev_len; + uint8_t len; + uint8_t id; + uint8_t crc; +} __attribute__ ((packed)); + +struct sysparam_context { + uint32_t addr; + struct entry_header entry; + uint64_t unused_keys[2]; size_t compactable; - uint8_t key_id; uint8_t max_key_id; - bool length_error; -} sysparam_context_t; - -//#define CHECK_FLASH_OP(x) if ((x) != SPI_FLASH_RESULT_OK) return SYSPARAM_ERR_IO; -#include -#define CHECK_FLASH_OP(x) do { int __x = (x); if ((__x) != SPI_FLASH_RESULT_OK) { printf("FLASH ERR: %s: %d\n", #x, __x); return SYSPARAM_ERR_IO; } } while (0); +}; /* Internal data structures */ -static struct { - uint32_t cur_addr; - uint32_t alt_addr; +struct { + uint32_t cur_base; + uint32_t alt_base; + uint32_t end_addr; size_t region_size; } sysparam_info; @@ -50,223 +67,156 @@ static sysparam_status_t format_region(uint32_t addr) { return SYSPARAM_OK; } -static sysparam_status_t write_region_header(uint32_t addr) { - uint32_t magic = SYSPARAM_MAGIC; +static inline sysparam_status_t write_region_header(uint32_t addr, bool active) { + uint32_t magic = active ? SYSPARAM_MAGIC : SYSPARAM_STALEMAGIC; + debug(3, "write region header (0x%08x) @ 0x%08x", magic, addr); CHECK_FLASH_OP(sdk_spi_flash_write(addr, &magic, 4)); return SYSPARAM_OK; } -static void init_context(sysparam_context_t *ctx) { +static void init_context(struct sysparam_context *ctx) { memset(ctx, 0, sizeof(*ctx)); - ctx->key_addr = sysparam_info.cur_addr + SYSPARAM_REGION_HEADER_SIZE; + ctx->addr = sysparam_info.cur_base; +} + +static sysparam_status_t init_write_context(struct sysparam_context *ctx) { + memset(ctx, 0, sizeof(*ctx)); + ctx->addr = sysparam_info.end_addr; + debug(3, "read entry header @ 0x%08x", ctx->addr); + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr, &ctx->entry, ENTRY_HEADER_SIZE)); + return SYSPARAM_OK; +} + +static sysparam_status_t find_entry(struct sysparam_context *ctx, uint8_t match_id) { + uint8_t prev_len; + + while (ctx->addr + ENTRY_SIZE(ctx->entry.len) < sysparam_info.end_addr) { + prev_len = ctx->entry.len; + ctx->addr += ENTRY_SIZE(ctx->entry.len); + + debug(3, "read entry header @ 0x%08x", ctx->addr); + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr, &ctx->entry, ENTRY_HEADER_SIZE)); + if (ctx->entry.prev_len != prev_len) { + // Uh oh.. This should match the entry.len field from the + // previous entry. If it doesn't, it means that field may have + // been corrupted and we don't even know if we're in the right + // place anymore. We have to bail out. + debug(1, "prev_len mismatch at 0x%08x (%d != %d)", ctx->addr, ctx->entry.prev_len, prev_len); + ctx->addr = sysparam_info.end_addr; + return SYSPARAM_ERR_CORRUPT; + } + + if (ctx->entry.id) { + if (!(ctx->entry.id & 0x80)) { + // Key definition + ctx->max_key_id = ctx->entry.id; + ctx->unused_keys[ctx->entry.id >> 6] |= (1 << (ctx->entry.id & 0x3f)); + if (!match_id) { + // We're looking for any key, so make this a matching key. + match_id = ctx->entry.id; + } + } else { + // Value entry + ctx->unused_keys[(ctx->entry.id >> 6) & 1] &= ~(1 << (ctx->entry.id & 0x3f)); + } + if (ctx->entry.id == match_id) { + return SYSPARAM_OK; + } + } else { + // Deleted entry + ctx->compactable += ENTRY_SIZE(ctx->entry.len); + } + } + ctx->entry.len = 0; + ctx->entry.id = 0; + return SYSPARAM_NOTFOUND; +} + +static inline sysparam_status_t read_payload(struct sysparam_context *ctx, uint8_t *buffer, size_t buffer_size) { + debug(3, "read payload (%d) @ 0x%08x", min(buffer_size, ctx->entry.len), ctx->addr); + CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr + ENTRY_HEADER_SIZE, buffer, min(buffer_size, ctx->entry.len))); + //FIXME: check crc + return SYSPARAM_OK; } // Scan through the region to find the location of the key entry matching the // specified name. -static sysparam_status_t find_key(sysparam_context_t *ctx, const char *key, size_t key_len, uint8_t *buffer) { - size_t payload_len; - uint8_t entry_id; - - // Are we already at the end? - if (ctx->key_addr == ctx->end_addr) return SYSPARAM_NOTFOUND; - - while (ctx->key_addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { - if (ctx->length_error) { - // Our last entry's length fields didn't match, which means we have - // no idea whether we're in the right place anymore. Can't - // continue. - //FIXME: print an error - return SYSPARAM_ERR_BADDATA; - } - if (ctx->key_len) { - ctx->key_addr += ctx->key_len + ENTRY_OVERHEAD; - } - - CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr, buffer, 2)); - payload_len = buffer[0]; - entry_id = buffer[1]; - ctx->key_len = payload_len; - - CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + payload_len + 2, buffer, 2)); - // checksum = buffer[0]; //FIXME - if (buffer[1] != payload_len) { - ctx->length_error = true; - } - - if (entry_id == 0xff) { - // End of entries - break; - } else if (!entry_id) { - // Deleted entry - } else if (!(entry_id & 0x80)) { - // Key definition - ctx->max_key_id = max(ctx->max_key_id, entry_id); - if (!key) { - ctx->key_id = entry_id; - return SYSPARAM_OK; - } - if (payload_len == key_len) { - CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + 2, buffer, key_len)); - //FIXME: check checksum - // If checksum checks out, clear length_error - if (!memcmp(key, buffer, key_len)) { - ctx->key_id = entry_id; - return SYSPARAM_OK; - } - } - } - } - ctx->end_addr = ctx->key_addr; - ctx->key_len = 0; - return SYSPARAM_NOTFOUND; -} - -// Scan through the region to find the location of the value entry matching the -// key found by `find_key` -static sysparam_status_t find_value(sysparam_context_t *ctx) { - uint32_t addr = ctx->key_addr; - size_t payload_len = ctx->key_len; - bool length_error = ctx->length_error; - uint8_t value_id = ctx->key_id | 0x80; - uint8_t entry_id; - uint8_t buffer[2]; - - while (addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { - if (length_error) { - // Our last entry's length fields didn't match, which means we have - // no idea whether we're in the right place anymore. Can't - // continue. - //FIXME: print an error - return SYSPARAM_ERR_BADDATA; - } - addr += payload_len + ENTRY_OVERHEAD; - - CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, 2)); - payload_len = buffer[0]; - entry_id = buffer[1]; - - CHECK_FLASH_OP(sdk_spi_flash_read(addr + payload_len + 2, buffer, 2)); - //checksum = buffer[0]; //FIXME - if (buffer[1] != payload_len) { - length_error = true; - } - - if (entry_id == 0xff) { - // End of entries - break; - } else if (!entry_id) { - // Deleted entry - } else if (!(entry_id & 0x80)) { - // Key entry. Make sure we update max_key_id. - ctx->max_key_id = max(ctx->max_key_id, entry_id); - } else if (entry_id == value_id) { - // We found our value - ctx->value_addr = addr; - ctx->value_len = payload_len; - return SYSPARAM_OK; - } - } - ctx->end_addr = addr; - ctx->value_len = 0; - return SYSPARAM_NOTFOUND; -} - -// Scan through any remaining data in the region until we get to the end, -// updating ctx as we go. -// NOTE: This assumes you've already called `find_key`/`find_value` (if you -// care about those things). It only looks for the end. -static sysparam_status_t find_end(sysparam_context_t *ctx) { - uint32_t addr; - size_t payload_len; - bool length_error = ctx->length_error; - uint8_t entry_id; - uint8_t buffer[2]; - - if (ctx->end_addr) { - return SYSPARAM_OK; - } - if (ctx->value_addr) { - addr = ctx->value_addr; - payload_len = ctx->value_len; - } else { - addr = ctx->key_addr; - payload_len = ctx->key_len; - } - while (addr < sysparam_info.cur_addr + sysparam_info.region_size - 2) { - if (length_error) { - // Our last entry's length fields didn't match, which means we have - // no idea whether we're in the right place anymore. Can't - // continue. - //FIXME: print an error - return SYSPARAM_ERR_BADDATA; - } - addr += payload_len + ENTRY_OVERHEAD; - - CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, 2)); - payload_len = buffer[0]; - entry_id = buffer[1]; - - CHECK_FLASH_OP(sdk_spi_flash_read(addr + payload_len + 2, buffer, 2)); - //checksum = buffer[0]; //FIXME - if (buffer[1] != payload_len) { - length_error = true; - } - - if (entry_id == 0xff) { - // End of entries - break; - } else if (!entry_id) { - // Deleted entry - } else if (!(entry_id & 0x80)) { - // Key entry. Make sure we update max_key_id. - ctx->max_key_id = max(ctx->max_key_id, entry_id); - } - } - ctx->end_addr = addr; - ctx->value_len = 0; - return SYSPARAM_OK; -} - -static sysparam_status_t read_key(sysparam_context_t *ctx, char *buffer, size_t buffer_size) { - if (!ctx->key_len) { - return SYSPARAM_NOTFOUND; - } - CHECK_FLASH_OP(sdk_spi_flash_read(ctx->key_addr + 2, buffer, min(buffer_size, ctx->key_len))); - //FIXME: check checksum - return SYSPARAM_OK; -} - -static sysparam_status_t read_value(sysparam_context_t *ctx, uint8_t *buffer, size_t buffer_size) { - if (!ctx->value_len) { - return SYSPARAM_NOTFOUND; - } - CHECK_FLASH_OP(sdk_spi_flash_read(ctx->value_addr + 2, buffer, min(buffer_size, ctx->value_len))); - //FIXME: check checksum - return SYSPARAM_OK; -} - -static inline sysparam_status_t write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, size_t payload_len) { - uint8_t buffer[2]; - - buffer[0] = payload_len; - buffer[1] = id; - CHECK_FLASH_OP(sdk_spi_flash_write(addr, buffer, 2)); - CHECK_FLASH_OP(sdk_spi_flash_write(addr + 2, payload, payload_len)); - buffer[0] = 0; //FIXME: calculate checksum - buffer[1] = payload_len; - CHECK_FLASH_OP(sdk_spi_flash_write(addr + 2 + payload_len, buffer, 2)); - - return SYSPARAM_OK; -} - -static sysparam_status_t compact_params(sysparam_context_t *ctx, bool delete_current_value) { - uint32_t new_base = sysparam_info.alt_addr; +static sysparam_status_t find_key(struct sysparam_context *ctx, const char *key, uint8_t key_len, uint8_t *buffer) { sysparam_status_t status; - uint32_t addr = new_base + SYSPARAM_REGION_HEADER_SIZE; - uint8_t current_key_id = 0; - uint32_t zero = 0; - sysparam_iter_t iter; + while (true) { + // Find the next key entry + status = find_entry(ctx, 0); + if (status != SYSPARAM_OK) return status; + if (!key) { + // We're looking for the next (any) key, so we're done. + break; + } + if (ctx->entry.len == key_len) { + status = read_payload(ctx, buffer, key_len); + if (status < 0) return status; + if (!memcmp(key, buffer, key_len)) { + // We have a match + break; + } + } + } + + return SYSPARAM_OK; +} + +static inline sysparam_status_t write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, uint8_t len, uint8_t prev_len) { + struct entry_header entry; + + debug(2, "Writing entry 0x%02x @ 0x%08x", id, addr); + entry.prev_len = prev_len; + entry.len = len; + entry.id = id; + entry.crc = 0; //FIXME: calculate crc + debug(3, "write entry header @ 0x%08x", addr); + CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); + debug(3, "write payload (%d) @ 0x%08x", len, addr + ENTRY_HEADER_SIZE); + CHECK_FLASH_OP(sdk_spi_flash_write(addr + ENTRY_HEADER_SIZE, payload, len)); + + return SYSPARAM_OK; +} + +static inline sysparam_status_t write_entry_tail(uint32_t addr, uint8_t prev_len) { + struct entry_header entry; + + entry.prev_len = prev_len; + entry.len = 0xff; + entry.id = 0xff; + entry.crc = 0xff; + debug(3, "write entry tail @ 0x%08x", addr); + CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); + + return SYSPARAM_OK; +} + +static inline sysparam_status_t delete_entry(uint32_t addr) { + struct entry_header entry; + + debug(2, "Deleting entry @ 0x%08x", addr); + debug(3, "read entry header @ 0x%08x", addr); + CHECK_FLASH_OP(sdk_spi_flash_read(addr, &entry, ENTRY_HEADER_SIZE)); + // Set the ID to zero to mark it as "deleted" + entry.id = 0x00; + debug(3, "write entry header @ 0x%08x", addr); + CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); + + return SYSPARAM_OK; +} + +static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *key_id) { + uint32_t new_base = sysparam_info.alt_base; + sysparam_status_t status; + uint32_t addr = new_base + REGION_HEADER_SIZE; + uint8_t current_key_id = 0; + sysparam_iter_t iter; + uint8_t prev_len = 0; + + debug(1, "compacting region (current size %d, expect to recover %d%s bytes)...", sysparam_info.end_addr - sysparam_info.cur_base, ctx->compactable, (ctx->unused_keys[0] || ctx->unused_keys[1]) ? "+ (unused keys present)" : ""); status = format_region(new_base); if (status < 0) return status; status = sysparam_iter_start(&iter); @@ -274,121 +224,191 @@ static sysparam_status_t compact_params(sysparam_context_t *ctx, bool delete_cur while (true) { status = sysparam_iter_next(&iter); - if (status < 0) break; + if (status != SYSPARAM_OK) break; current_key_id++; - if (iter.ctx->key_addr == ctx->key_addr) { - ctx->key_addr = addr; - } - if (iter.ctx->key_id == ctx->key_id) { - ctx->key_id = current_key_id; - } - // Write the key to the new region - status = write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len); + debug(2, "writing %d key @ 0x%08x", current_key_id, addr); + status = write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len, prev_len); if (status < 0) break; - addr += iter.key_len + ENTRY_OVERHEAD; + prev_len = iter.key_len; + addr += ENTRY_SIZE(iter.key_len); - if (iter.ctx->value_addr == ctx->value_addr) { - if (delete_current_value) { - // Don't copy the old value, since we'll just be deleting it - // and writing a new one as soon as we return. - ctx->value_addr = 0; - ctx->value_len = 0; - continue; - } else { - ctx->value_addr = addr; - } + if (iter.ctx->entry.id == *key_id) { + // Update key_id to have the correct id for the compacted result + *key_id = current_key_id; + // Don't copy the old value, since we'll just be deleting it + // and writing a new one as soon as we return. + continue; } + // Copy the value to the new region - status = write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len); + debug(2, "writing %d value @ 0x%08x", current_key_id, addr); + status = write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len, prev_len); if (status < 0) break; - addr += iter.value_len + ENTRY_OVERHEAD; + prev_len = iter.value_len; + addr += ENTRY_SIZE(iter.value_len); } sysparam_iter_end(&iter); - // If we broke out with an error, return the error instead of continuing. - if (status < 0) return status; + if (status >= 0) { + status = write_entry_tail(addr, prev_len); + } - // Fix up all the remaining bits of ctx to have correct values for the new - // (compacted) region. - ctx->end_addr = addr; - ctx->compactable = 0; - ctx->max_key_id = current_key_id; - ctx->length_error = false; + // If we broke out with an error, return the error instead of continuing. + if (status < 0) { + debug(1, "error encountered during compacting (%d)", status); + return status; + } // Switch to officially using the new region. - status = write_region_header(new_base); + status = write_region_header(new_base, true); if (status < 0) return status; - sysparam_info.alt_addr = sysparam_info.cur_addr; - sysparam_info.cur_addr = new_base; - // Zero out the old header, so future calls to sysparam_init() will know - // that the new one is the correct region to be looking at. - CHECK_FLASH_OP(sdk_spi_flash_write(sysparam_info.alt_addr, &zero, 4)); + status = write_region_header(sysparam_info.cur_base, false); + if (status < 0) return status; + + sysparam_info.alt_base = sysparam_info.cur_base; + sysparam_info.cur_base = new_base; + sysparam_info.end_addr = addr; + + // Fix up ctx so it doesn't point to invalid stuff + memset(ctx, 0, sizeof(*ctx)); + ctx->addr = addr; + ctx->entry.prev_len = prev_len; + ctx->max_key_id = current_key_id; + + debug(1, "done compacting (current size %d)", sysparam_info.end_addr - sysparam_info.cur_base); return SYSPARAM_OK; } /* Public Functions */ -sysparam_status_t sysparam_init(uint32_t base_addr, bool create) { +sysparam_status_t sysparam_init(uint32_t base_addr) { sysparam_status_t status; - size_t region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; - uint32_t magic; - int i; + uint32_t magic0, magic1; + struct sysparam_context ctx; - sysparam_info.region_size = region_size; - for (i = 0; i < 2; i++) { - CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + (i * region_size), &magic, 4)); - if (magic == SYSPARAM_MAGIC) { - sysparam_info.cur_addr = base_addr + i * region_size; - sysparam_info.alt_addr = base_addr + (i ^ 1) * region_size; - return SYSPARAM_OK; - } - } - // We couldn't find one already present, so create a new one at the - // specified address (if `create` is set). - if (create) { - sysparam_info.cur_addr = base_addr; - sysparam_info.alt_addr = base_addr + region_size; - status = format_region(base_addr); - if (status != SYSPARAM_OK) return status; - return write_region_header(base_addr); + sysparam_info.region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; + // First, see if we can find an existing one. + debug(3, "read magic @ 0x%08x", base_addr); + CHECK_FLASH_OP(sdk_spi_flash_read(base_addr, &magic0, 4)); + debug(3, "read magic @ 0x%08x", base_addr + sysparam_info.region_size); + CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + sysparam_info.region_size, &magic1, 4)); + if (magic0 == SYSPARAM_MAGIC && magic1 == SYSPARAM_STALEMAGIC) { + // Sysparam area found, first region is active + sysparam_info.cur_base = base_addr; + sysparam_info.alt_base = base_addr + sysparam_info.region_size; + } else if (magic0 == SYSPARAM_STALEMAGIC && magic1 == SYSPARAM_MAGIC) { + // Sysparam area found, second region is active + sysparam_info.cur_base = base_addr + sysparam_info.region_size; + sysparam_info.alt_base = base_addr; + } else if (magic0 == SYSPARAM_MAGIC && magic1 == SYSPARAM_MAGIC) { + // Both regions are marked as active. Not sure which to use. + // This can theoretically happen if something goes wrong at exactly the + // wrong time during compacting. + return SYSPARAM_ERR_CORRUPT; + } else if (magic0 == SYSPARAM_STALEMAGIC && magic1 == SYSPARAM_STALEMAGIC) { + // Both regions are marked as inactive. This shouldn't ever happen. + return SYSPARAM_ERR_CORRUPT; } else { + // Looks like there's something else at that location entirely. return SYSPARAM_NOTFOUND; } + + // Find the actual end + sysparam_info.end_addr = sysparam_info.cur_base + sysparam_info.region_size; + init_context(&ctx); + status = find_entry(&ctx, 0xff); + if (status < 0) { + sysparam_info.cur_base = 0; + sysparam_info.alt_base = 0; + sysparam_info.end_addr = 0; + return status; + } + if (status == SYSPARAM_OK) { + sysparam_info.end_addr = ctx.addr; + } + + return SYSPARAM_OK; } -sysparam_status_t sysparam_get_raw(const char *key, uint8_t **destptr, size_t *actual_length) { - sysparam_context_t ctx; +sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force) { + sysparam_status_t status; + uint32_t buffer[SCAN_BUFFER_SIZE]; + uint32_t addr; + int i; + size_t region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; + + if (!force) { + // First, scan through the area and make sure it's actually empty and + // we're not going to be clobbering something else important. + for (addr = base_addr; addr < base_addr + SYSPARAM_REGION_SECTORS * 2 * sdk_flashchip.sector_size; addr += SCAN_BUFFER_SIZE) { + debug(3, "read %d words @ 0x%08x", SCAN_BUFFER_SIZE, addr); + CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, SCAN_BUFFER_SIZE * 4)); + for (i = 0; i < SCAN_BUFFER_SIZE; i++) { + if (buffer[i] != 0xffffffff) { + // Uh oh, not empty. + return SYSPARAM_NOTFOUND; + } + } + } + } + + if (sysparam_info.cur_base == base_addr || sysparam_info.alt_base == base_addr) { + // We're reformating the same region we're already using. + // De-initialize everything to force the caller to do a clean + // `sysparam_init()` afterwards. + memset(&sysparam_info, 0, sizeof(sysparam_info)); + } + status = format_region(base_addr); + if (status < 0) return status; + status = format_region(base_addr + region_size); + if (status < 0) return status; + status = write_entry_tail(base_addr + REGION_HEADER_SIZE, 0); + if (status < 0) return status; + status = write_region_header(base_addr + region_size, false); + if (status < 0) return status; + status = write_region_header(base_addr, true); + if (status < 0) return status; + + return SYSPARAM_OK; +} + +sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length) { + struct sysparam_context ctx; sysparam_status_t status; size_t key_len = strlen(key); - uint8_t *buffer = malloc(key_len + 2); + uint8_t *buffer; + + if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + buffer = malloc(key_len + 2); if (!buffer) return SYSPARAM_ERR_NOMEM; do { init_context(&ctx); status = find_key(&ctx, key, key_len, buffer); if (status != SYSPARAM_OK) break; - status = find_value(&ctx); + // Find the associated value + status = find_entry(&ctx, ctx.entry.id | 0x80); if (status != SYSPARAM_OK) break; - buffer = realloc(buffer, ctx.value_len + 1); + buffer = realloc(buffer, ctx.entry.len + 1); if (!buffer) { return SYSPARAM_ERR_NOMEM; } - status = read_value(&ctx, buffer, ctx.value_len); + status = read_payload(&ctx, buffer, ctx.entry.len); if (status != SYSPARAM_OK) break; // Zero-terminate the result, just in case (doesn't hurt anything for // non-string data, and can avoid nasty mistakes if the caller wants to // interpret the result as a string). - buffer[ctx.value_len] = 0; + buffer[ctx.entry.len] = 0; *destptr = buffer; - if (actual_length) *actual_length = ctx.value_len; + if (actual_length) *actual_length = ctx.entry.len; return SYSPARAM_OK; } while (false); @@ -398,17 +418,13 @@ sysparam_status_t sysparam_get_raw(const char *key, uint8_t **destptr, size_t *a return status; } -sysparam_status_t sysparam_get_string(const char *key, char **destptr) { - // `sysparam_get_raw` will zero-terminate the result as a matter of course, - // so no need to do that here. - return sysparam_get_raw(key, (uint8_t **)destptr, NULL); -} - -sysparam_status_t sysparam_get_raw_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length) { - sysparam_context_t ctx; +sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length) { + struct sysparam_context ctx; sysparam_status_t status = SYSPARAM_OK; size_t key_len = strlen(key); + if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + // Supplied buffer must be at least as large as the key, or 2 bytes, // whichever is larger. if (buffer_size < max(key_len, 2)) return SYSPARAM_ERR_NOMEM; @@ -418,26 +434,89 @@ sysparam_status_t sysparam_get_raw_static(const char *key, uint8_t *buffer, size init_context(&ctx); status = find_key(&ctx, key, key_len, buffer); if (status != SYSPARAM_OK) return status; - status = find_value(&ctx); + status = find_entry(&ctx, ctx.entry.id | 0x80); if (status != SYSPARAM_OK) return status; - status = read_value(&ctx, buffer, buffer_size); + status = read_payload(&ctx, buffer, buffer_size); if (status != SYSPARAM_OK) return status; - if (actual_length) *actual_length = ctx.value_len; + if (actual_length) *actual_length = ctx.entry.len; return SYSPARAM_OK; } -sysparam_status_t sysparam_put_raw(const char *key, const uint8_t *value, size_t value_len) { - sysparam_context_t ctx; +sysparam_status_t sysparam_get_string(const char *key, char **destptr) { + // `sysparam_get_data` will zero-terminate the result as a matter of course, + // so no need to do that here. + return sysparam_get_data(key, (uint8_t **)destptr, NULL); +} + +sysparam_status_t sysparam_get_int(const char *key, int32_t *result) { + char buffer[32]; + size_t len; + char *endptr; + int32_t value; + sysparam_status_t status; + + status = sysparam_get_data_static(key, (uint8_t *)buffer, 12, &len); + if (status != SYSPARAM_OK) return status; + if (len > 31) return SYSPARAM_PARSEFAILED; + buffer[len] = 0; + value = strtol(buffer, &endptr, 0); + if (*endptr) return SYSPARAM_PARSEFAILED; + + *result = value; + return SYSPARAM_OK; +} + +sysparam_status_t sysparam_get_bool(const char *key, bool *result) { + char buffer[6]; + size_t len; + int i; + sysparam_status_t status; + + status = sysparam_get_data_static(key, (uint8_t *)buffer, 12, &len); + if (status != SYSPARAM_OK) return status; + if (len > 5) return SYSPARAM_PARSEFAILED; + buffer[len] = 0; + for (i = 0; i < len; i++) { + // Quick and dirty tolower(). Not perfect, but works for our purposes, + // and avoids needing to pull in additional libc modules. + if (buffer[i] >= 0x41) buffer[i] |= 0x20; + } + if (!strcmp(buffer, "t")) { + *result = true; + } else if (!strcmp(buffer, "f")) { + *result = false; + } else if (!strcmp(buffer, "true")) { + *result = true; + } else if (!strcmp(buffer, "false")) { + *result = false; + } else if (!strcmp(buffer, "0")) { + *result = true; + } else if (!strcmp(buffer, "1")) { + *result = false; + } else { + return SYSPARAM_PARSEFAILED; + } + return SYSPARAM_OK; +} + +sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len) { + struct sysparam_context ctx; + struct sysparam_context write_ctx; sysparam_status_t status = SYSPARAM_OK; - size_t key_len = strlen(key); + uint8_t key_len = strlen(key); uint8_t *buffer; size_t free_space; size_t needed_space; bool free_value = false; + uint8_t key_id = 0; + uint32_t old_value_addr = 0; - if (!key_len) return SYSPARAM_ERR_BADARGS; - if (value_len && (value & 0x3)) { + if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + if (!key_len) return SYSPARAM_ERR_BADVALUE; + if (value_len > 0xff) return SYSPARAM_ERR_BADVALUE; + + if (value_len && ((intptr_t)value & 0x3)) { // The passed value isn't word-aligned. This will be a problem later // when calling `sdk_spi_flash_write`, so make a word-aligned copy. buffer = malloc(value_len); @@ -449,109 +528,160 @@ sysparam_status_t sysparam_put_raw(const char *key, const uint8_t *value, size_t // Create a working buffer for `find_key` to use. buffer = malloc(key_len); if (!buffer) { - if (free_value) free(value); + if (free_value) free((void *)value); return SYSPARAM_ERR_NOMEM; } - do { init_context(&ctx); status = find_key(&ctx, key, key_len, buffer); if (status == SYSPARAM_OK) { // Key already exists, see if there's a current value. - status = find_value(&ctx); + key_id = ctx.entry.id; + status = find_entry(&ctx, key_id | 0x80); + if (status == SYSPARAM_OK) { + old_value_addr = ctx.addr; + } } if (status < 0) break; if (value_len) { - if (ctx.value_len == value_len) { - // Are we trying to write the same value that's already there? - if (value_len > key_len) { - buffer = realloc(buffer, value_len); - if (!buffer) return SYSPARAM_ERR_NOMEM; + if (old_value_addr) { + if (ctx.entry.len == value_len) { + // Are we trying to write the same value that's already there? + if (value_len > key_len) { + buffer = realloc(buffer, value_len); + if (!buffer) return SYSPARAM_ERR_NOMEM; + } + status = read_payload(&ctx, buffer, value_len); + if (status == SYSPARAM_ERR_CORRUPT) { + // If the CRC check failed, don't worry about it. We're + // going to be deleting this entry anyway. + } else if (status < 0) { + break; + } else if (!memcmp(buffer, value, value_len)) { + // Yup, it's a match! No need to do anything further, + // just leave the current value as-is. + status = SYSPARAM_OK; + break; + } } - status = read_value(&ctx, buffer, value_len); - if (status < 0) break; - if (!memcmp(buffer, value, value_len)) { - // Yup! No need to do anything further, just leave the - // current value as-is. - status = SYSPARAM_OK; - break; - } - } - // Write the new/updated value - status = find_end(&ctx); - if (status < 0) break; - - // Since we will be deleting the old value (if any) make sure that - // the compactable count includes the space taken up by that entry - // too (even though it's not actually deleted yet) - if (ctx.value_addr) { - ctx.compactable += ctx.value_len + ENTRY_OVERHEAD; + // Since we will be deleting the old value (if any) make sure + // that the compactable count includes the space taken up by + // that entry too (even though it's not actually deleted yet) + ctx.compactable += ENTRY_SIZE(ctx.entry.len); } // Append new value to the end, but first make sure we have enough // space. - free_space = sysparam_info.region_size - (ctx.end_addr - sysparam_info.cur_addr); - needed_space = ENTRY_OVERHEAD + value_len; - if (!ctx.key_id) { - // We did not find a previous key entry matching this key. We will - // need to add a key entry as well. + free_space = sysparam_info.cur_base + sysparam_info.region_size - sysparam_info.end_addr - 4; + needed_space = ENTRY_SIZE(value_len); + if (!key_id) { + // We did not find a previous key entry matching this key. We + // will need to add a key entry as well. key_len = strlen(key); - needed_space += ENTRY_OVERHEAD + key_len; + needed_space += ENTRY_SIZE(key_len); } if (needed_space > free_space) { - if (needed_space > free_space + ctx.compactable) { - // Nothing we can do here.. We're full. - // (at least full enough that compacting won't help us store - // this value) - //FIXME: debug message - status = SYSPARAM_ERR_FULL; - break; + // Can we compact things? + // First, scan all remaining entries up to the end so we can + // get a reasonably accurate "compactable" reading. + find_entry(&ctx, 0xff); + if (needed_space <= free_space + ctx.compactable) { + // We should be able to get enough space by compacting. + status = compact_params(&ctx, &key_id); + if (status < 0) break; + old_value_addr = 0; + } else if (ctx.unused_keys[0] || ctx.unused_keys[1]) { + // Compacting will gain more space than expected, because + // there are some keys that can be omitted too, but we + // don't know exactly how much that will gain, so all we + // can do is give it a try and see if it gives us enough. + status = compact_params(&ctx, &key_id); + if (status < 0) break; + old_value_addr = 0; } - // We should have enough space after we compact things. - status = compact_params(&ctx, true); - if (status < 0) break; + free_space = sysparam_info.cur_base + sysparam_info.region_size - sysparam_info.end_addr - 4; + } + if (needed_space > free_space) { + // Nothing we can do here.. We're full. + // (at least full enough that compacting won't help us store + // this value) + debug(1, "region full (need %d of %d remaining)", needed_space, free_space); + status = SYSPARAM_ERR_FULL; + break; } - if (!ctx.key_id) { - // Write key entry for new key - ctx.key_id = ctx.max_key_id + 1; - if (ctx.key_id > MAX_KEY_ID) { - status = SYSPARAM_ERR_FULL; - break; + init_write_context(&write_ctx); + + if (!key_id) { + // We need to write a key entry for a new key. + // If we didn't find the key, then we already know find_entry + // has gone through the entire contents, and thus + // ctx.max_key_id has the largest key_id found in the whole + // region. + key_id = ctx.max_key_id + 1; + if (key_id > MAX_KEY_ID) { + if (ctx.unused_keys[0] || ctx.unused_keys[1]) { + status = compact_params(&ctx, &key_id); + if (status < 0) break; + old_value_addr = 0; + } else { + debug(1, "out of ids!"); + status = SYSPARAM_ERR_FULL; + break; + } } - status = write_entry(ctx.end_addr, ctx.key_id, (uint8_t *)key, key_len); + status = write_entry(write_ctx.addr, key_id, (uint8_t *)key, key_len, write_ctx.entry.prev_len); if (status < 0) break; - ctx.end_addr += key_len + ENTRY_OVERHEAD; + write_ctx.addr += ENTRY_SIZE(key_len); + write_ctx.entry.prev_len = key_len; } // Write new value - status = write_entry(ctx.end_addr, ctx.key_id | 0x80, value, value_len); + status = write_entry(write_ctx.addr, key_id | 0x80, value, value_len, write_ctx.entry.prev_len); if (status < 0) break; + write_ctx.addr += ENTRY_SIZE(value_len); + status = write_entry_tail(write_ctx.addr, value_len); + if (status < 0) break; + sysparam_info.end_addr = write_ctx.addr; } // Delete old value (if present) by setting it's id to 0x00 - if (ctx.value_addr) { - buffer[0] = 0; - if (sdk_spi_flash_write(ctx.value_addr + 1, buffer, 1) != SPI_FLASH_RESULT_OK) { - status = SYSPARAM_ERR_IO; - break; - } + if (old_value_addr) { + status = delete_entry(old_value_addr); + if (status < 0) break; } } while (false); - if (free_value) free(value); + if (free_value) free((void *)value); free(buffer); return status; } -sysparam_status_t sysparam_put_string(const char *key, const char *value) { - return sysparam_put_raw(key, (const uint8_t *)value, strlen(value)); +sysparam_status_t sysparam_set_string(const char *key, const char *value) { + return sysparam_set_data(key, (const uint8_t *)value, strlen(value)); +} + +sysparam_status_t sysparam_set_int(const char *key, int32_t value) { + uint8_t buffer[12]; + int len; + + len = snprintf((char *)buffer, 12, "%d", value); + return sysparam_set_data(key, buffer, len); +} + +sysparam_status_t sysparam_set_bool(const char *key, bool value) { + uint8_t buf[4] = {0xff, 0xff, 0xff, 0xff}; + + buf[0] = value ? 't' : 'f'; + return sysparam_set_data(key, buf, 1); } sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { + if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + iter->bufsize = DEFAULT_ITER_BUF_SIZE; iter->key = malloc(iter->bufsize); if (!iter->key) { @@ -560,7 +690,7 @@ sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { } iter->key_len = 0; iter->value_len = 0; - iter->ctx = malloc(sizeof(sysparam_context_t)); + iter->ctx = malloc(sizeof(struct sysparam_context)); if (!iter->ctx) { free(iter->key); iter->bufsize = 0; @@ -575,15 +705,22 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { uint8_t buffer[2]; sysparam_status_t status; size_t required_len; + struct sysparam_context *ctx = iter->ctx; + struct sysparam_context value_ctx; + size_t key_space; while (true) { - status = find_key(iter->ctx, NULL, 0, buffer); - if (status != 0) return status; - status = find_value(iter->ctx); + status = find_key(ctx, NULL, 0, buffer); + if (status != SYSPARAM_OK) return status; + memcpy(&value_ctx, ctx, sizeof(value_ctx)); + + status = find_entry(&value_ctx, ctx->entry.id | 0x80); if (status < 0) return status; if (status == SYSPARAM_NOTFOUND) continue; - required_len = iter->ctx->key_len + 1 + iter->ctx->value_len + 1; - if (required_len < iter->bufsize) { + + key_space = ROUND_TO_WORD_BOUNDARY(ctx->entry.len + 1); + required_len = key_space + value_ctx.entry.len + 1; + if (required_len > iter->bufsize) { iter->key = realloc(iter->key, required_len); if (!iter->key) { iter->bufsize = 0; @@ -592,18 +729,19 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { iter->bufsize = required_len; } - status = read_key(iter->ctx, iter->key, iter->bufsize); + status = read_payload(ctx, (uint8_t *)iter->key, iter->bufsize); if (status < 0) return status; // Null-terminate the key - iter->key[iter->ctx->key_len] = 0; - iter->key_len = iter->ctx->key_len; + iter->key[ctx->entry.len] = 0; + iter->key_len = ctx->entry.len; - iter->value = (uint8_t *)iter->key + iter->ctx->key_len + 1; - status = read_value(iter->ctx, iter->value, iter->bufsize - iter->ctx->key_len - 1); + iter->value = (uint8_t *)(iter->key + key_space); + status = read_payload(&value_ctx, iter->value, iter->bufsize - key_space); if (status < 0) return status; // Null-terminate the value (just in case) - iter->value[iter->ctx->value_len] = 0; - iter->value_len = iter->ctx->value_len; + iter->value[value_ctx.entry.len] = 0; + iter->value_len = value_ctx.entry.len; + debug(2, "iter_next: (0x%08x) '%s' = (0x%08x) '%s' (%d)", ctx->addr, iter->key, value_ctx.addr, iter->value, iter->value_len); return SYSPARAM_OK; } From 40266a1e63e9a45068ff4caca4ef4738ad7e8aa9 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Mon, 7 Mar 2016 09:47:27 -0800 Subject: [PATCH 03/11] Add sysparam_editor example --- examples/sysparam_editor/Makefile | 14 ++ examples/sysparam_editor/sysparam_editor.c | 158 +++++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 examples/sysparam_editor/Makefile create mode 100644 examples/sysparam_editor/sysparam_editor.c diff --git a/examples/sysparam_editor/Makefile b/examples/sysparam_editor/Makefile new file mode 100644 index 0000000..a7843f3 --- /dev/null +++ b/examples/sysparam_editor/Makefile @@ -0,0 +1,14 @@ +PROGRAM=sysparam_editor + +# Setting this to 1..3 will add extra debugging output to stdout +EXTRA_CFLAGS=-DSYSPARAM_DEBUG=0 + +include ../../common.mk + +# `make dump-flash` can be used to view the current contents of the sysparam +# regions in flash. +dump-flash: + esptool.py read_flash 0x1fa000 4096 r1.bin + hexdump -C r1.bin + esptool.py read_flash 0x1fb000 4096 r2.bin + hexdump -C r2.bin diff --git a/examples/sysparam_editor/sysparam_editor.c b/examples/sysparam_editor/sysparam_editor.c new file mode 100644 index 0000000..ac8fee1 --- /dev/null +++ b/examples/sysparam_editor/sysparam_editor.c @@ -0,0 +1,158 @@ +#include "FreeRTOS.h" +#include "task.h" +#include +#include +#include +#include + +#define CMD_BUF_SIZE 512 + +// This is just below the upper-4 sdk-reserved sectors for a 16mbit flash +// Note that the sysparam area will take up two sectors (0x1FAxxx and 0x1FBxxx) +#define SYSPARAM_ADDR 0x1fa000 + +const int status_base = -6; +const char *status_messages[] = { + "SYSPARAM_ERR_NOMEM", + "SYSPARAM_ERR_CORRUPT", + "SYSPARAM_ERR_IO", + "SYSPARAM_ERR_FULL", + "SYSPARAM_ERR_BADVALUE", + "SYSPARAM_ERR_NOINIT", + "SYSPARAM_OK", + "SYSPARAM_NOTFOUND", + "SYSPARAM_PARSEFAILED", +}; + +void usage(void) { + printf( + "Available commands:\n" + " ? -- Query the value of \n" + " = -- Set to \n" + " dump -- Show all currently set keys/values\n" + " reformat -- Reinitialize (clear) the sysparam area\n" + " help -- Show this help screen\n" + ); +} + +size_t tty_readline(char *buffer, size_t buf_size, bool echo) { + size_t i = 0; + int c; + + while (true) { + c = getchar(); + if (c == '\r') { + if (echo) putchar('\n'); + break; + } else if (c == '\b' || c == 0x7f) { + if (i) { + if (echo) printf("\b \b"); + i--; + } + } else if (c < 0x20) { + /* Ignore other control characters */ + } else if (i >= buf_size - 1) { + if (echo) putchar('\a'); + } else { + buffer[i++] = c; + if (echo) putchar(c); + } + } + + buffer[i] = 0; + return i; +} + +sysparam_status_t dump_params(void) { + sysparam_status_t status; + sysparam_iter_t iter; + + status = sysparam_iter_start(&iter); + if (status < 0) return status; + while (true) { + status = sysparam_iter_next(&iter); + if (status != SYSPARAM_OK) break; + printf(" %s=%s\n", iter.key, iter.value); + } + sysparam_iter_end(&iter); + + if (status == SYSPARAM_NOTFOUND) { + // This is the normal status when we've reached the end of all entries. + return SYSPARAM_OK; + } else { + // Something apparently went wrong + return status; + } +} + +void sysparam_editor_task(void *pvParameters) { + char *cmd_buffer = malloc(CMD_BUF_SIZE); + sysparam_status_t status; + char *value; + size_t len; + + if (!cmd_buffer) { + printf("ERROR: Cannot allocate command buffer!\n"); + return; + } + + // NOTE: Eventually, this initialization part will be done automatically on + // system startup, so the app won't need to do it. + printf("Initializing sysparam...\n"); + status = sysparam_init(SYSPARAM_ADDR); + printf("(status %d)\n", status); + if (status == SYSPARAM_NOTFOUND) { + printf("Trying to create new sysparam area...\n"); + status = sysparam_create_area(SYSPARAM_ADDR, false); + printf("(status %d)\n", status); + if (status == SYSPARAM_OK) { + status = sysparam_init(SYSPARAM_ADDR); + printf("(status %d)\n", status); + } + } + + while (true) { + printf("==> "); + len = tty_readline(cmd_buffer, CMD_BUF_SIZE, true); + status = 0; + if (!len) continue; + if (cmd_buffer[len - 1] == '?') { + cmd_buffer[len - 1] = 0; + printf("Querying '%s'...\n", cmd_buffer); + status = sysparam_get_string(cmd_buffer, &value); + if (status == SYSPARAM_OK) { + printf(" '%s' = '%s'\n", cmd_buffer, value); + } + free(value); + } else if ((value = strchr(cmd_buffer, '='))) { + *value++ = 0; + printf("Setting '%s' to '%s'...\n", cmd_buffer, value); + status = sysparam_set_string(cmd_buffer, value); + } else if (!strcmp(cmd_buffer, "dump")) { + printf("Dumping all params:\n"); + status = dump_params(); + } else if (!strcmp(cmd_buffer, "reformat")) { + printf("Re-initializing region...\n"); + status = sysparam_create_area(SYSPARAM_ADDR, true); + if (status == SYSPARAM_OK) { + // We need to re-init after wiping out the region we've been + // using. + status = sysparam_init(SYSPARAM_ADDR); + } + } else if (!strcmp(cmd_buffer, "help")) { + usage(); + } else { + printf("Unrecognized command.\n\n"); + usage(); + } + + if (status != SYSPARAM_OK) { + printf("! Operation returned status: %d (%s)\n", status, status_messages[status - status_base]); + } + } +} + +void user_init(void) +{ + xTaskCreate(sysparam_editor_task, (signed char *)"sysparam_editor_task", 512, NULL, 2, NULL); +} From b67783635be0ede5a8fc524b7d66d8a4b4a79142 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Tue, 8 Mar 2016 12:31:29 -0800 Subject: [PATCH 04/11] Sysparam code cleanup --- core/sysparam.c | 345 ++++++++++++++++++++++++++++-------------------- 1 file changed, 202 insertions(+), 143 deletions(-) diff --git a/core/sysparam.c b/core/sysparam.c index d6fd4b1..8f9415d 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -5,28 +5,60 @@ #include //TODO: make this properly threadsafe -//FIXME: reduce stack usage -//TODO: make stderr work for debug output +//TODO: reduce stack usage +//FIXME: CRC calculations/checking + +/* The "magic" values that indicate the start of a sysparam region in flash. + * Note that SYSPARAM_STALE_MAGIC is written over SYSPARAM_ACTIVE_MAGIC, so it + * must not contain any set bits which are not set in SYSPARAM_ACTIVE_MAGIC + * (that is, going from SYSPARAM_ACTIVE_MAGIC to SYSPARAM_STALE_MAGIC must only + * clear bits, not set them) + */ +#define SYSPARAM_ACTIVE_MAGIC 0x70524f45 // "EORp" in little-endian +#define SYSPARAM_STALE_MAGIC 0x40524f45 // "EOR@" in little-endian + +/* The size of the initial buffer created by sysparam_iter_start, etc, to hold + * returned key-value pairs. Setting this too small may result in a lot of + * unnecessary reallocs. Setting it too large will waste memory when iterating + * through entries. + */ +#define DEFAULT_ITER_BUF_SIZE 64 + +/* The size of the buffer (in words) used by `sysparam_create_area` when + * scanning a potential area to make sure it's currently empty. Note that this + * space is taken from the stack, so it should not be too large. + */ +#define SCAN_BUFFER_SIZE 8 // words + +/* Size of region/entry headers. These should not normally need tweaking (and + * will probably require some code changes if they are tweaked). + */ +#define REGION_HEADER_SIZE 4 // NOTE: Must be multiple of 4 +#define ENTRY_HEADER_SIZE 4 // NOTE: Must be multiple of 4 + +/* Maximum value that can be used for a key_id. This is limited by the format + * to 0x7e (0x7f would produce a corresponding value ID of 0xff, which is + * invalid) + */ +#define MAX_KEY_ID 0x7e #ifndef SYSPARAM_DEBUG #define SYSPARAM_DEBUG 0 #endif -#define debug(level, format, ...) if (SYSPARAM_DEBUG >= (level)) { printf("%s" format "\n", "sysparam: ", ## __VA_ARGS__); } - -#define SYSPARAM_MAGIC 0x70524f45 // "EORp" in little-endian -#define SYSPARAM_STALEMAGIC 0x40524f45 // "EOR@" in little-endian -#define REGION_HEADER_SIZE 4 // NOTE: Must be multiple of 4 -#define ENTRY_HEADER_SIZE 4 // NOTE: Must be multiple of 4 -#define DEFAULT_ITER_BUF_SIZE 64 -#define MAX_KEY_ID 0x7e -#define SCAN_BUFFER_SIZE 8 // words +/******************************* Useful Macros *******************************/ #define ROUND_TO_WORD_BOUNDARY(x) (((x) + 3) & 0xfffffffc) #define ENTRY_SIZE(payload_len) (ENTRY_HEADER_SIZE + ROUND_TO_WORD_BOUNDARY(payload_len)) +#define max(x, y) ((x) > (y) ? (x) : (y)) +#define min(x, y) ((x) < (y) ? (x) : (y)) + +#define debug(level, format, ...) if (SYSPARAM_DEBUG >= (level)) { printf("%s" format "\n", "sysparam: ", ## __VA_ARGS__); } + #define CHECK_FLASH_OP(x) do { int __x = (x); if ((__x) != SPI_FLASH_RESULT_OK) { debug(1, "FLASH ERR: %d", __x); return SYSPARAM_ERR_IO; } } while (0); +/********************* Internal datatypes and structures *********************/ struct entry_header { uint8_t prev_len; @@ -43,21 +75,19 @@ struct sysparam_context { uint8_t max_key_id; }; -/* Internal data structures */ +/*************************** Global variables/data ***************************/ -struct { +static struct { uint32_t cur_base; uint32_t alt_base; uint32_t end_addr; size_t region_size; -} sysparam_info; +} _sysparam_info; -/* Internal routines */ +/***************************** Internal routines *****************************/ -#define max(x, y) ((x) > (y) ? (x) : (y)) -#define min(x, y) ((x) < (y) ? (x) : (y)) - -static sysparam_status_t format_region(uint32_t addr) { +/** Erase the sectors of a region */ +static sysparam_status_t _format_region(uint32_t addr) { uint16_t sector = addr / sdk_flashchip.sector_size; int i; @@ -67,30 +97,38 @@ static sysparam_status_t format_region(uint32_t addr) { return SYSPARAM_OK; } -static inline sysparam_status_t write_region_header(uint32_t addr, bool active) { - uint32_t magic = active ? SYSPARAM_MAGIC : SYSPARAM_STALEMAGIC; +/** Write the magic value at the beginning of a region */ +static inline sysparam_status_t _write_region_header(uint32_t addr, bool active) { + uint32_t magic = active ? SYSPARAM_ACTIVE_MAGIC : SYSPARAM_STALE_MAGIC; debug(3, "write region header (0x%08x) @ 0x%08x", magic, addr); CHECK_FLASH_OP(sdk_spi_flash_write(addr, &magic, 4)); return SYSPARAM_OK; } -static void init_context(struct sysparam_context *ctx) { +/** Initialize a context structure at the beginning of the active region */ +static void _init_context(struct sysparam_context *ctx) { memset(ctx, 0, sizeof(*ctx)); - ctx->addr = sysparam_info.cur_base; + ctx->addr = _sysparam_info.cur_base; } +/** Initialize a context structure at the end of the active region */ static sysparam_status_t init_write_context(struct sysparam_context *ctx) { memset(ctx, 0, sizeof(*ctx)); - ctx->addr = sysparam_info.end_addr; + ctx->addr = _sysparam_info.end_addr; debug(3, "read entry header @ 0x%08x", ctx->addr); CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr, &ctx->entry, ENTRY_HEADER_SIZE)); return SYSPARAM_OK; } -static sysparam_status_t find_entry(struct sysparam_context *ctx, uint8_t match_id) { +/** Search through the region for an entry matching the specified id + * + * @param match_id The id to match, or 0 to match any key, or 0xff to scan + * to the end. + */ +static sysparam_status_t _find_entry(struct sysparam_context *ctx, uint8_t match_id) { uint8_t prev_len; - while (ctx->addr + ENTRY_SIZE(ctx->entry.len) < sysparam_info.end_addr) { + while (ctx->addr + ENTRY_SIZE(ctx->entry.len) < _sysparam_info.end_addr) { prev_len = ctx->entry.len; ctx->addr += ENTRY_SIZE(ctx->entry.len); @@ -102,7 +140,7 @@ static sysparam_status_t find_entry(struct sysparam_context *ctx, uint8_t match_ // been corrupted and we don't even know if we're in the right // place anymore. We have to bail out. debug(1, "prev_len mismatch at 0x%08x (%d != %d)", ctx->addr, ctx->entry.prev_len, prev_len); - ctx->addr = sysparam_info.end_addr; + ctx->addr = _sysparam_info.end_addr; return SYSPARAM_ERR_CORRUPT; } @@ -132,28 +170,28 @@ static sysparam_status_t find_entry(struct sysparam_context *ctx, uint8_t match_ return SYSPARAM_NOTFOUND; } -static inline sysparam_status_t read_payload(struct sysparam_context *ctx, uint8_t *buffer, size_t buffer_size) { +/** Read the payload from the current entry pointed to by `ctx` */ +static inline sysparam_status_t _read_payload(struct sysparam_context *ctx, uint8_t *buffer, size_t buffer_size) { debug(3, "read payload (%d) @ 0x%08x", min(buffer_size, ctx->entry.len), ctx->addr); CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr + ENTRY_HEADER_SIZE, buffer, min(buffer_size, ctx->entry.len))); //FIXME: check crc return SYSPARAM_OK; } -// Scan through the region to find the location of the key entry matching the -// specified name. -static sysparam_status_t find_key(struct sysparam_context *ctx, const char *key, uint8_t key_len, uint8_t *buffer) { +/** Find the entry corresponding to the specified key name */ +static sysparam_status_t _find_key(struct sysparam_context *ctx, const char *key, uint8_t key_len, uint8_t *buffer) { sysparam_status_t status; while (true) { // Find the next key entry - status = find_entry(ctx, 0); + status = _find_entry(ctx, 0); if (status != SYSPARAM_OK) return status; if (!key) { // We're looking for the next (any) key, so we're done. break; } if (ctx->entry.len == key_len) { - status = read_payload(ctx, buffer, key_len); + status = _read_payload(ctx, buffer, key_len); if (status < 0) return status; if (!memcmp(key, buffer, key_len)) { // We have a match @@ -165,7 +203,8 @@ static sysparam_status_t find_key(struct sysparam_context *ctx, const char *key, return SYSPARAM_OK; } -static inline sysparam_status_t write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, uint8_t len, uint8_t prev_len) { +/** Write an entry at the specified address */ +static inline sysparam_status_t _write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, uint8_t len, uint8_t prev_len) { struct entry_header entry; debug(2, "Writing entry 0x%02x @ 0x%08x", id, addr); @@ -181,7 +220,10 @@ static inline sysparam_status_t write_entry(uint32_t addr, uint8_t id, const uin return SYSPARAM_OK; } -static inline sysparam_status_t write_entry_tail(uint32_t addr, uint8_t prev_len) { +/** Write the "tail" entry on the end of the region + * (this entry just contains the `prev_len` field with all others set to 0xff) + */ +static inline sysparam_status_t _write_entry_tail(uint32_t addr, uint8_t prev_len) { struct entry_header entry; entry.prev_len = prev_len; @@ -194,7 +236,8 @@ static inline sysparam_status_t write_entry_tail(uint32_t addr, uint8_t prev_len return SYSPARAM_OK; } -static inline sysparam_status_t delete_entry(uint32_t addr) { +/** Mark an entry as "deleted" so it won't be considered in future reads */ +static inline sysparam_status_t _delete_entry(uint32_t addr) { struct entry_header entry; debug(2, "Deleting entry @ 0x%08x", addr); @@ -208,16 +251,28 @@ static inline sysparam_status_t delete_entry(uint32_t addr) { return SYSPARAM_OK; } -static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *key_id) { - uint32_t new_base = sysparam_info.alt_base; +/** Compact the current region, removing all deleted/unused entries, and write + * the result to the alternate region, then make the new alternate region the + * active one. + * + * @param key_id A pointer to the "current" key ID. + * + * NOTE: The value corresponding to the passed key ID will not be written to + * the output (because it is assumed it will be overwritten as the next step + * in `sysparam_set_data` anyway). When compacting, this routine will + * automatically update *key_id to contain the ID of this key in the new + * compacted result as well. + */ +static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t *key_id) { + uint32_t new_base = _sysparam_info.alt_base; sysparam_status_t status; uint32_t addr = new_base + REGION_HEADER_SIZE; uint8_t current_key_id = 0; sysparam_iter_t iter; uint8_t prev_len = 0; - debug(1, "compacting region (current size %d, expect to recover %d%s bytes)...", sysparam_info.end_addr - sysparam_info.cur_base, ctx->compactable, (ctx->unused_keys[0] || ctx->unused_keys[1]) ? "+ (unused keys present)" : ""); - status = format_region(new_base); + debug(1, "compacting region (current size %d, expect to recover %d%s bytes)...", _sysparam_info.end_addr - _sysparam_info.cur_base, ctx->compactable, (ctx->unused_keys[0] || ctx->unused_keys[1]) ? "+ (unused keys present)" : ""); + status = _format_region(new_base); if (status < 0) return status; status = sysparam_iter_start(&iter); if (status < 0) return status; @@ -230,7 +285,7 @@ static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *k // Write the key to the new region debug(2, "writing %d key @ 0x%08x", current_key_id, addr); - status = write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len, prev_len); + status = _write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len, prev_len); if (status < 0) break; prev_len = iter.key_len; addr += ENTRY_SIZE(iter.key_len); @@ -245,7 +300,7 @@ static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *k // Copy the value to the new region debug(2, "writing %d value @ 0x%08x", current_key_id, addr); - status = write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len, prev_len); + status = _write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len, prev_len); if (status < 0) break; prev_len = iter.value_len; addr += ENTRY_SIZE(iter.value_len); @@ -253,7 +308,7 @@ static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *k sysparam_iter_end(&iter); if (status >= 0) { - status = write_entry_tail(addr, prev_len); + status = _write_entry_tail(addr, prev_len); } // If we broke out with an error, return the error instead of continuing. @@ -263,14 +318,14 @@ static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *k } // Switch to officially using the new region. - status = write_region_header(new_base, true); + status = _write_region_header(new_base, true); if (status < 0) return status; - status = write_region_header(sysparam_info.cur_base, false); + status = _write_region_header(_sysparam_info.cur_base, false); if (status < 0) return status; - sysparam_info.alt_base = sysparam_info.cur_base; - sysparam_info.cur_base = new_base; - sysparam_info.end_addr = addr; + _sysparam_info.alt_base = _sysparam_info.cur_base; + _sysparam_info.cur_base = new_base; + _sysparam_info.end_addr = addr; // Fix up ctx so it doesn't point to invalid stuff memset(ctx, 0, sizeof(*ctx)); @@ -278,38 +333,38 @@ static sysparam_status_t compact_params(struct sysparam_context *ctx, uint8_t *k ctx->entry.prev_len = prev_len; ctx->max_key_id = current_key_id; - debug(1, "done compacting (current size %d)", sysparam_info.end_addr - sysparam_info.cur_base); + debug(1, "done compacting (current size %d)", _sysparam_info.end_addr - _sysparam_info.cur_base); return SYSPARAM_OK; } -/* Public Functions */ +/***************************** Public Functions ******************************/ sysparam_status_t sysparam_init(uint32_t base_addr) { sysparam_status_t status; uint32_t magic0, magic1; struct sysparam_context ctx; - sysparam_info.region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; + _sysparam_info.region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; // First, see if we can find an existing one. debug(3, "read magic @ 0x%08x", base_addr); CHECK_FLASH_OP(sdk_spi_flash_read(base_addr, &magic0, 4)); - debug(3, "read magic @ 0x%08x", base_addr + sysparam_info.region_size); - CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + sysparam_info.region_size, &magic1, 4)); - if (magic0 == SYSPARAM_MAGIC && magic1 == SYSPARAM_STALEMAGIC) { + debug(3, "read magic @ 0x%08x", base_addr + _sysparam_info.region_size); + CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + _sysparam_info.region_size, &magic1, 4)); + if (magic0 == SYSPARAM_ACTIVE_MAGIC && magic1 == SYSPARAM_STALE_MAGIC) { // Sysparam area found, first region is active - sysparam_info.cur_base = base_addr; - sysparam_info.alt_base = base_addr + sysparam_info.region_size; - } else if (magic0 == SYSPARAM_STALEMAGIC && magic1 == SYSPARAM_MAGIC) { + _sysparam_info.cur_base = base_addr; + _sysparam_info.alt_base = base_addr + _sysparam_info.region_size; + } else if (magic0 == SYSPARAM_STALE_MAGIC && magic1 == SYSPARAM_ACTIVE_MAGIC) { // Sysparam area found, second region is active - sysparam_info.cur_base = base_addr + sysparam_info.region_size; - sysparam_info.alt_base = base_addr; - } else if (magic0 == SYSPARAM_MAGIC && magic1 == SYSPARAM_MAGIC) { + _sysparam_info.cur_base = base_addr + _sysparam_info.region_size; + _sysparam_info.alt_base = base_addr; + } else if (magic0 == SYSPARAM_ACTIVE_MAGIC && magic1 == SYSPARAM_ACTIVE_MAGIC) { // Both regions are marked as active. Not sure which to use. // This can theoretically happen if something goes wrong at exactly the // wrong time during compacting. return SYSPARAM_ERR_CORRUPT; - } else if (magic0 == SYSPARAM_STALEMAGIC && magic1 == SYSPARAM_STALEMAGIC) { + } else if (magic0 == SYSPARAM_STALE_MAGIC && magic1 == SYSPARAM_STALE_MAGIC) { // Both regions are marked as inactive. This shouldn't ever happen. return SYSPARAM_ERR_CORRUPT; } else { @@ -318,17 +373,17 @@ sysparam_status_t sysparam_init(uint32_t base_addr) { } // Find the actual end - sysparam_info.end_addr = sysparam_info.cur_base + sysparam_info.region_size; - init_context(&ctx); - status = find_entry(&ctx, 0xff); + _sysparam_info.end_addr = _sysparam_info.cur_base + _sysparam_info.region_size; + _init_context(&ctx); + status = _find_entry(&ctx, 0xff); if (status < 0) { - sysparam_info.cur_base = 0; - sysparam_info.alt_base = 0; - sysparam_info.end_addr = 0; + _sysparam_info.cur_base = 0; + _sysparam_info.alt_base = 0; + _sysparam_info.end_addr = 0; return status; } if (status == SYSPARAM_OK) { - sysparam_info.end_addr = ctx.addr; + _sysparam_info.end_addr = ctx.addr; } return SYSPARAM_OK; @@ -356,21 +411,21 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force) { } } - if (sysparam_info.cur_base == base_addr || sysparam_info.alt_base == base_addr) { + if (_sysparam_info.cur_base == base_addr || _sysparam_info.alt_base == base_addr) { // We're reformating the same region we're already using. // De-initialize everything to force the caller to do a clean // `sysparam_init()` afterwards. - memset(&sysparam_info, 0, sizeof(sysparam_info)); + memset(&_sysparam_info, 0, sizeof(_sysparam_info)); } - status = format_region(base_addr); + status = _format_region(base_addr); if (status < 0) return status; - status = format_region(base_addr + region_size); + status = _format_region(base_addr + region_size); if (status < 0) return status; - status = write_entry_tail(base_addr + REGION_HEADER_SIZE, 0); + status = _write_entry_tail(base_addr + REGION_HEADER_SIZE, 0); if (status < 0) return status; - status = write_region_header(base_addr + region_size, false); + status = _write_region_header(base_addr + region_size, false); if (status < 0) return status; - status = write_region_header(base_addr, true); + status = _write_region_header(base_addr, true); if (status < 0) return status; return SYSPARAM_OK; @@ -382,24 +437,24 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * size_t key_len = strlen(key); uint8_t *buffer; - if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; buffer = malloc(key_len + 2); if (!buffer) return SYSPARAM_ERR_NOMEM; do { - init_context(&ctx); - status = find_key(&ctx, key, key_len, buffer); + _init_context(&ctx); + status = _find_key(&ctx, key, key_len, buffer); if (status != SYSPARAM_OK) break; // Find the associated value - status = find_entry(&ctx, ctx.entry.id | 0x80); + status = _find_entry(&ctx, ctx.entry.id | 0x80); if (status != SYSPARAM_OK) break; buffer = realloc(buffer, ctx.entry.len + 1); if (!buffer) { return SYSPARAM_ERR_NOMEM; } - status = read_payload(&ctx, buffer, ctx.entry.len); + status = _read_payload(&ctx, buffer, ctx.entry.len); if (status != SYSPARAM_OK) break; // Zero-terminate the result, just in case (doesn't hurt anything for @@ -413,7 +468,6 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * } while (false); free(buffer); - *destptr = NULL; if (actual_length) *actual_length = 0; return status; } @@ -423,7 +477,7 @@ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, siz sysparam_status_t status = SYSPARAM_OK; size_t key_len = strlen(key); - if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; // Supplied buffer must be at least as large as the key, or 2 bytes, // whichever is larger. @@ -431,12 +485,12 @@ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, siz if (actual_length) *actual_length = 0; - init_context(&ctx); - status = find_key(&ctx, key, key_len, buffer); + _init_context(&ctx); + status = _find_key(&ctx, key, key_len, buffer); if (status != SYSPARAM_OK) return status; - status = find_entry(&ctx, ctx.entry.id | 0x80); + status = _find_entry(&ctx, ctx.entry.id | 0x80); if (status != SYSPARAM_OK) return status; - status = read_payload(&ctx, buffer, buffer_size); + status = _read_payload(&ctx, buffer, buffer_size); if (status != SYSPARAM_OK) return status; if (actual_length) *actual_length = ctx.entry.len; @@ -450,54 +504,59 @@ sysparam_status_t sysparam_get_string(const char *key, char **destptr) { } sysparam_status_t sysparam_get_int(const char *key, int32_t *result) { - char buffer[32]; - size_t len; + char *buffer; char *endptr; int32_t value; sysparam_status_t status; - status = sysparam_get_data_static(key, (uint8_t *)buffer, 12, &len); + status = sysparam_get_string(key, &buffer); if (status != SYSPARAM_OK) return status; - if (len > 31) return SYSPARAM_PARSEFAILED; - buffer[len] = 0; value = strtol(buffer, &endptr, 0); - if (*endptr) return SYSPARAM_PARSEFAILED; + if (*endptr) { + // There was extra crap at the end of the string. + free(buffer); + return SYSPARAM_PARSEFAILED; + } *result = value; + free(buffer); return SYSPARAM_OK; } sysparam_status_t sysparam_get_bool(const char *key, bool *result) { - char buffer[6]; - size_t len; + char *buffer; int i; sysparam_status_t status; - status = sysparam_get_data_static(key, (uint8_t *)buffer, 12, &len); + status = sysparam_get_string(key, &buffer); if (status != SYSPARAM_OK) return status; - if (len > 5) return SYSPARAM_PARSEFAILED; - buffer[len] = 0; - for (i = 0; i < len; i++) { - // Quick and dirty tolower(). Not perfect, but works for our purposes, - // and avoids needing to pull in additional libc modules. - if (buffer[i] >= 0x41) buffer[i] |= 0x20; - } - if (!strcmp(buffer, "t")) { - *result = true; - } else if (!strcmp(buffer, "f")) { - *result = false; - } else if (!strcmp(buffer, "true")) { - *result = true; - } else if (!strcmp(buffer, "false")) { - *result = false; - } else if (!strcmp(buffer, "0")) { - *result = true; - } else if (!strcmp(buffer, "1")) { - *result = false; - } else { - return SYSPARAM_PARSEFAILED; - } - return SYSPARAM_OK; + do { + for (i = 0; buffer[i]; i++) { + // Quick-and-dirty tolower(). Not perfect, but works for our + // purposes, and avoids needing to pull in additional libc stuff. + if (buffer[i] >= 0x41) buffer[i] |= 0x20; + } + if (!strcmp(buffer, "y") || + !strcmp(buffer, "yes") || + !strcmp(buffer, "t") || + !strcmp(buffer, "true") || + !strcmp(buffer, "1")) { + *result = true; + break; + } + if (!strcmp(buffer, "n") || + !strcmp(buffer, "no") || + !strcmp(buffer, "f") || + !strcmp(buffer, "false") || + !strcmp(buffer, "0")) { + *result = false; + break; + } + status = SYSPARAM_PARSEFAILED; + } while (0); + + free(buffer); + return status; } sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len) { @@ -512,7 +571,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ uint8_t key_id = 0; uint32_t old_value_addr = 0; - if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; if (!key_len) return SYSPARAM_ERR_BADVALUE; if (value_len > 0xff) return SYSPARAM_ERR_BADVALUE; @@ -525,7 +584,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ value = buffer; free_value = true; } - // Create a working buffer for `find_key` to use. + // Create a working buffer for `_find_key` to use. buffer = malloc(key_len); if (!buffer) { if (free_value) free((void *)value); @@ -533,12 +592,12 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ } do { - init_context(&ctx); - status = find_key(&ctx, key, key_len, buffer); + _init_context(&ctx); + status = _find_key(&ctx, key, key_len, buffer); if (status == SYSPARAM_OK) { // Key already exists, see if there's a current value. key_id = ctx.entry.id; - status = find_entry(&ctx, key_id | 0x80); + status = _find_entry(&ctx, key_id | 0x80); if (status == SYSPARAM_OK) { old_value_addr = ctx.addr; } @@ -553,7 +612,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ buffer = realloc(buffer, value_len); if (!buffer) return SYSPARAM_ERR_NOMEM; } - status = read_payload(&ctx, buffer, value_len); + status = _read_payload(&ctx, buffer, value_len); if (status == SYSPARAM_ERR_CORRUPT) { // If the CRC check failed, don't worry about it. We're // going to be deleting this entry anyway. @@ -575,7 +634,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ // Append new value to the end, but first make sure we have enough // space. - free_space = sysparam_info.cur_base + sysparam_info.region_size - sysparam_info.end_addr - 4; + free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4; needed_space = ENTRY_SIZE(value_len); if (!key_id) { // We did not find a previous key entry matching this key. We @@ -587,10 +646,10 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ // Can we compact things? // First, scan all remaining entries up to the end so we can // get a reasonably accurate "compactable" reading. - find_entry(&ctx, 0xff); + _find_entry(&ctx, 0xff); if (needed_space <= free_space + ctx.compactable) { // We should be able to get enough space by compacting. - status = compact_params(&ctx, &key_id); + status = _compact_params(&ctx, &key_id); if (status < 0) break; old_value_addr = 0; } else if (ctx.unused_keys[0] || ctx.unused_keys[1]) { @@ -598,11 +657,11 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ // there are some keys that can be omitted too, but we // don't know exactly how much that will gain, so all we // can do is give it a try and see if it gives us enough. - status = compact_params(&ctx, &key_id); + status = _compact_params(&ctx, &key_id); if (status < 0) break; old_value_addr = 0; } - free_space = sysparam_info.cur_base + sysparam_info.region_size - sysparam_info.end_addr - 4; + free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4; } if (needed_space > free_space) { // Nothing we can do here.. We're full. @@ -617,14 +676,14 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ if (!key_id) { // We need to write a key entry for a new key. - // If we didn't find the key, then we already know find_entry + // If we didn't find the key, then we already know _find_entry // has gone through the entire contents, and thus // ctx.max_key_id has the largest key_id found in the whole // region. key_id = ctx.max_key_id + 1; if (key_id > MAX_KEY_ID) { if (ctx.unused_keys[0] || ctx.unused_keys[1]) { - status = compact_params(&ctx, &key_id); + status = _compact_params(&ctx, &key_id); if (status < 0) break; old_value_addr = 0; } else { @@ -633,24 +692,24 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ break; } } - status = write_entry(write_ctx.addr, key_id, (uint8_t *)key, key_len, write_ctx.entry.prev_len); + status = _write_entry(write_ctx.addr, key_id, (uint8_t *)key, key_len, write_ctx.entry.prev_len); if (status < 0) break; write_ctx.addr += ENTRY_SIZE(key_len); write_ctx.entry.prev_len = key_len; } // Write new value - status = write_entry(write_ctx.addr, key_id | 0x80, value, value_len, write_ctx.entry.prev_len); + status = _write_entry(write_ctx.addr, key_id | 0x80, value, value_len, write_ctx.entry.prev_len); if (status < 0) break; write_ctx.addr += ENTRY_SIZE(value_len); - status = write_entry_tail(write_ctx.addr, value_len); + status = _write_entry_tail(write_ctx.addr, value_len); if (status < 0) break; - sysparam_info.end_addr = write_ctx.addr; + _sysparam_info.end_addr = write_ctx.addr; } // Delete old value (if present) by setting it's id to 0x00 if (old_value_addr) { - status = delete_entry(old_value_addr); + status = _delete_entry(old_value_addr); if (status < 0) break; } } while (false); @@ -675,12 +734,12 @@ sysparam_status_t sysparam_set_int(const char *key, int32_t value) { sysparam_status_t sysparam_set_bool(const char *key, bool value) { uint8_t buf[4] = {0xff, 0xff, 0xff, 0xff}; - buf[0] = value ? 't' : 'f'; + buf[0] = value ? 'y' : 'n'; return sysparam_set_data(key, buf, 1); } sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { - if (!sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; iter->bufsize = DEFAULT_ITER_BUF_SIZE; iter->key = malloc(iter->bufsize); @@ -696,7 +755,7 @@ sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { iter->bufsize = 0; return SYSPARAM_ERR_NOMEM; } - init_context(iter->ctx); + _init_context(iter->ctx); return SYSPARAM_OK; } @@ -710,11 +769,11 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { size_t key_space; while (true) { - status = find_key(ctx, NULL, 0, buffer); + status = _find_key(ctx, NULL, 0, buffer); if (status != SYSPARAM_OK) return status; memcpy(&value_ctx, ctx, sizeof(value_ctx)); - status = find_entry(&value_ctx, ctx->entry.id | 0x80); + status = _find_entry(&value_ctx, ctx->entry.id | 0x80); if (status < 0) return status; if (status == SYSPARAM_NOTFOUND) continue; @@ -729,14 +788,14 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { iter->bufsize = required_len; } - status = read_payload(ctx, (uint8_t *)iter->key, iter->bufsize); + status = _read_payload(ctx, (uint8_t *)iter->key, iter->bufsize); if (status < 0) return status; // Null-terminate the key iter->key[ctx->entry.len] = 0; iter->key_len = ctx->entry.len; iter->value = (uint8_t *)(iter->key + key_space); - status = read_payload(&value_ctx, iter->value, iter->bufsize - key_space); + status = _read_payload(&value_ctx, iter->value, iter->bufsize - key_space); if (status < 0) return status; // Null-terminate the value (just in case) iter->value[value_ctx.entry.len] = 0; From 7721e24b0e49d3f1c0733fd1f5a9f09a994e6c9c Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Tue, 8 Mar 2016 17:00:24 -0800 Subject: [PATCH 05/11] Add documentation to sysparam.h --- core/include/sysparam.h | 329 +++++++++++++++++++++++++++++++++++++++- core/sysparam.c | 9 ++ 2 files changed, 335 insertions(+), 3 deletions(-) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index 6f158ee..300d447 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -4,12 +4,20 @@ #include #ifndef SYSPARAM_REGION_SECTORS -// Number of (4K) sectors that make up a sysparam region. Total sysparam data -// cannot be larger than this. Note that the actual amount of used flash -// space will be *twice* this amount. +/** Number of (4K) sectors that make up a sysparam region. Total sysparam data + * cannot be larger than this. Note that the full sysparam area is two + * regions, so the actual amount of used flash space will be *twice* this + * amount. + */ #define SYSPARAM_REGION_SECTORS 1 #endif +/** Status codes returned by all sysparam functions + * + * Error codes all have values less than zero, and can be returned by any + * function. Values greater than zero are non-error status codes which may be + * returned by some functions to indicate various results (each function should document which non-error statuses it may return). + */ typedef enum { SYSPARAM_ERR_NOMEM = -6, SYSPARAM_ERR_CORRUPT = -5, @@ -22,6 +30,19 @@ typedef enum { SYSPARAM_PARSEFAILED = 2, } sysparam_status_t; + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_BADVALUE One or more arguments are invalid + * @retval SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + +/** Structure used by sysparam_iter_next() to keep track of its current state + * and return its results. This should be initialized by calling + * sysparam_iter_start() and cleaned up afterward by calling + * sysparam_iter_end(). + */ typedef struct { char *key; uint8_t *value; @@ -31,19 +52,321 @@ typedef struct { struct sysparam_context *ctx; } sysparam_iter_t; +/** Initialize sysparam and set up the current area of flash to use. + * + * This must be called (and return successfully) before any other sysparam + * routines (except sysparam_create_area()) are called. + * + * This should normally be taken care of automatically on boot by the OS + * startup routines. It may be necessary to call it specially, however, if + * the normal initialization failed, or after calling sysparam_create_area() + * to reformat the current area. + * + * @param base_addr [in] The flash address which should contain the start of + * the (already present) sysparam area + * + * @retval SYSPARAM_OK Initialization successful. + * @retval SYSPARAM_NOTFOUND The specified address does not appear to + * contain a sysparam area. It may be necessary + * to call sysparam_create_area() to create one + * first. + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_init(uint32_t base_addr); + +/** Create a new sysparam area in flash at the specified address. + * + * By default, this routine will scan the specified area to make sure it + * appears to be empty (i.e. all 0xFF bytes) before setting it up as a new + * sysparam area. If there appears to be other data already present, it will + * not overwrite it. Setting `force` to `true` will cause it to clobber any + * existing data instead. + * + * @param base_addr [in] The flash address at which it should start + * (must be a multiple of the sector size) + * @param force [in] Proceed even if the space does not appear to be empty + * + * @retval SYSPARAM_OK Area (re)created successfully. + * @retval SYSPARAM_NOTFOUND `force` was not specified, and the area at + * `base_addr` appears to have other data. No + * action taken. + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * + * Note: This routine can create a sysparam area in another location than the + * one currently being used, but does not change which area is currently used + * (you will need to call sysparam_init() again if you want to do that). If + * you reformat the area currently being used, you will also need to call + * sysparam_init() again afterward before you will be able to continue using + * it. + */ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); + +/** Get the value associated with a key + * + * This is the core "get value" function. It will retrieve the value for the + * specified key in a freshly malloc()'d buffer and return it. Raw values can + * contain any data (including zero bytes), so the `actual_length` parameter + * should be used to determine the length of the data in the buffer. + * + * Note: It is up to the caller to free() the returned buffer when done using + * it. + * + * @param key [in] Key name (zero-terminated string) + * @param destptr [out] Pointer to a location to hold the address of the + * returned data buffer + * @param actual_length [out] Pointer to a location to hold the length of the + * returned data buffer + * + * @retval SYSPARAM_OK Value successfully retrieved. + * @retval SYSPARAM_NOTFOUND Key/value not found. No buffer returned. + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * + * Note: If the result is anything other than SYSPARAM_OK, the value + * in `destptr` is not changed. This means it is possible to set a default + * value before calling this function which will be left as-is if a sysparam + * value could not be successfully read. + */ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length); + +/** Get the value associate with a key (static buffers only) + * + * This performs the same function as sysparam_get_data() but without + * performing any memory allocations. It can thus be used before the heap has + * been configured or in other cases where using the heap would be a problem + * (i.e. in an OOM handler, etc). It requires that the caller pass in a + * suitably sized buffer for the value to be read (if the supplied buffer is + * not large enough, the returned value will be truncated and the full + * required length will be returned in `actual_length`). + * + * NOTE: In addition to being large enough for the value, the supplied buffer + * must also be at least as large as the length of the key being requested. + * If it is not, an error will be returned. + * + * @param key [in] Key name (zero-terminated string) + * @param buffer [in] Pointer to a buffer to hold the returned value + * @param buffer_size [in] Length of the supplied buffer in bytes + * @param actual_length [out] pointer to a location to hold the actual length + * of the data which was associated with the key + * (may be NULL). + * + * @retval SYSPARAM_OK Value successfully retrieved + * @retval SYSPARAM_NOTFOUND Key/value not found + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_NOMEM The supplied buffer is too small + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); + +/** Return the string value associated with a key + * + * This routine can be used if you know that the value in a key will (or at + * least should) be a string. It will return a zero-terminated char buffer + * containing the value retrieved. + * + * Note: It is up to the caller to free() the returned buffer when done using + * it. + * + * @param key [in] Key name (zero-terminated string) + * @param destptr [out] Pointer to a location to hold the address of the + * returned data buffer + * + * @retval SYSPARAM_OK Value successfully retrieved. + * @retval SYSPARAM_NOTFOUND Key/value not found. + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * + * Note: If the result is anything other than SYSPARAM_OK, the value + * in `destptr` is not changed. This means it is possible to set a default + * value before calling this function which will be left as-is if a sysparam + * value could not be successfully read. + */ sysparam_status_t sysparam_get_string(const char *key, char **destptr); + +/** Return the int32_t value associated with a key + * + * This routine can be used if you know that the value in a key will (or at + * least should) be an integer value. It will parse the stored data as a + * number (in standard decimal or "0x" hex notation) and return the result. + * + * @param key [in] Key name (zero-terminated string) + * @param result [out] Pointer to a location to hold returned integer value + * + * @retval SYSPARAM_OK Value successfully retrieved. + * @retval SYSPARAM_NOTFOUND Key/value not found. + * @retval SYSPARAM_PARSEFAILED The retrieved value could not be parsed as an + * integer. + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * + * Note: If the result is anything other than SYSPARAM_OK, the value + * in `result` is not changed. This means it is possible to set a default + * value before calling this function which will be left as-is if a sysparam + * value could not be successfully read. + */ sysparam_status_t sysparam_get_int(const char *key, int32_t *result); + +/** Return the boolean value associated with a key + * + * This routine can be used if you know that the value in a key will (or at + * least should) be a boolean setting. It will read the specified value as a + * text string and attempt to parse it as a boolean value. + * + * It will recognize the following (case-insensitive) strings: + * * True: "yes", "y", "true", "t", "1" + * * False: "no", "n", "false", "f", "0" + * + * @param key [in] Key name (zero-terminated string) + * @param result [out] Pointer to a location to hold returned boolean value + * + * @retval SYSPARAM_OK Value successfully retrieved. + * @retval SYSPARAM_NOTFOUND Key/value not found. + * @retval SYSPARAM_PARSEFAILED The retrieved value could not be parsed as a + * boolean setting. + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * + * Note: If the result is anything other than SYSPARAM_OK, the value + * in `result` is not changed. This means it is possible to set a default + * value before calling this function which will be left as-is if a sysparam + * value could not be successfully read. + */ sysparam_status_t sysparam_get_bool(const char *key, bool *result); + +/** Set the value associated with a key + * + * The supplied value can be any data, up to 255 bytes in length. If `value` + * is NULL or `value_len` is 0, this is treated as a request to delete any + * current entry matching `key`. + * + * @param key [in] Key name (zero-terminated string) + * @param value [in] Pointer to a buffer containing the value data + * @param value_len [in] Length of the data in the buffer + * + * @retval SYSPARAM_OK Value successfully set. + * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval SYSPARAM_ERR_BADVALUE Either an empty key was provided or value_len + * is too large + * @retval SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len); + +/** Set a key's value from a string + * + * Performs the same function as sysparam_set_data(), but accepts a + * zero-terminated string value instead. + * + * @param key [in] Key name (zero-terminated string) + * @param value [in] Value to set (zero-terminated string) + * + * @retval SYSPARAM_OK Value successfully set. + * @retval SYSPARAM_ERR_BADVALUE Either an empty key was provided or the + * length of `value` is too large + * @retval SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_set_string(const char *key, const char *value); + +/** Set a key's value as a number + * + * Converts an int32_t value to a decimal number and writes it to the + * specified key. This does the inverse of the sysparam_get_int() + * function. + * + * @param key [in] Key name (zero-terminated string) + * @param value [in] Value to set + * + * @retval SYSPARAM_OK Value successfully set. + * @retval SYSPARAM_ERR_BADVALUE An empty key was provided. + * @retval SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_set_int(const char *key, int32_t value); + +/** Set a key's value as a boolean (yes/no) string + * + * Converts a bool value to a corresponding text string and writes it to the + * specified key. This does the inverse of the sysparam_get_bool() + * function. + * + * Note that if the key already contains a value which parses to the same + * boolean (true/false) value, it is left unchanged. + * + * @param key [in] Key name (zero-terminated string) + * @param value [in] Value to set + * + * @retval SYSPARAM_OK Value successfully set. + * @retval SYSPARAM_ERR_BADVALUE An empty key was provided. + * @retval SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_set_bool(const char *key, bool value); + +/** Begin iterating through all key/value pairs + * + * This function initializes a sysparam_iter_t structure to prepare it for + * iterating through the list of key/value pairs using sysparam_iter_next(). + * This does not fetch any items (the first successive call to + * sysparam_iter_next() will return the first key/value in the list). + * + * NOTE: When done, you must call sysparam_iter_end() to free the resources + * associated with `iter`, or you will leak memory. + * + * @param iter [in] A pointer to a sysparam_iter_t structure to initialize + * + * @retval SYSPARAM_OK Initialization successful + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + */ sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter); + +/** Fetch the next key/value pair + * + * This will retrieve the next key and value from the sysparam area, placing + * them in `iter->key`, and `iter->value` (and updating `iter->key_len` and + * `iter->value_len`). + * + * NOTE: `iter->key` and `iter->value` are static buffers local to the `iter` + * structure, and will be overwritten with the next call to + * sysparam_iter_next() using the same `iter`. They should *not* be free()d + * after use. + * + * @param iter [in] The iterator structure to update + * + * @retval SYSPARAM_OK Next key/value retrieved + * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + */ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter); + +/** Finish iterating through keys/values + * + * Cleans up and releases resources allocated by sysparam_iter_start() / + * sysparam_iter_next(). + */ void sysparam_iter_end(sysparam_iter_t *iter); #endif /* _SYSPARAM_H_ */ diff --git a/core/sysparam.c b/core/sysparam.c index 8f9415d..b664dbd 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -575,6 +575,8 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ if (!key_len) return SYSPARAM_ERR_BADVALUE; if (value_len > 0xff) return SYSPARAM_ERR_BADVALUE; + if (!value) value_len = 0; + if (value_len && ((intptr_t)value & 0x3)) { // The passed value isn't word-aligned. This will be a problem later // when calling `sdk_spi_flash_write`, so make a word-aligned copy. @@ -733,6 +735,13 @@ sysparam_status_t sysparam_set_int(const char *key, int32_t value) { sysparam_status_t sysparam_set_bool(const char *key, bool value) { uint8_t buf[4] = {0xff, 0xff, 0xff, 0xff}; + bool old_value; + + // Don't write anything if the current setting already evaluates to the + // same thing. + if (sysparam_get_bool(key, &old_value) == SYSPARAM_OK) { + if (old_value == value) return SYSPARAM_OK; + } buf[0] = value ? 'y' : 'n'; return sysparam_set_data(key, buf, 1); From 16f611358b443409defe82d5f1d9d7fbc25381d1 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Tue, 8 Mar 2016 17:48:48 -0800 Subject: [PATCH 06/11] Fix up sysparam.h docs --- core/include/sysparam.h | 304 ++++++++++++++++++++-------------------- 1 file changed, 152 insertions(+), 152 deletions(-) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index 300d447..b4b2337 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -1,6 +1,16 @@ #ifndef _SYSPARAM_H_ #define _SYSPARAM_H_ +/** @file sysparam.h + * + * Read/write "system parameters" to persistent flash. + * + * System parameters are stored as key/value pairs. Keys are string values + * between 1 and 255 characters long. Values can be any data up to 255 bytes + * in length (but are most commonly also strings). + * + */ + #include #ifndef SYSPARAM_REGION_SECTORS @@ -14,30 +24,22 @@ /** Status codes returned by all sysparam functions * - * Error codes all have values less than zero, and can be returned by any - * function. Values greater than zero are non-error status codes which may be - * returned by some functions to indicate various results (each function should document which non-error statuses it may return). + * Error codes (`SYSPARAM_ERR_*`) all have values less than zero, and can be + * returned by any function. Values greater than zero are non-error status + * codes which may be returned by some functions to indicate various results. */ typedef enum { - SYSPARAM_ERR_NOMEM = -6, - SYSPARAM_ERR_CORRUPT = -5, - SYSPARAM_ERR_IO = -4, - SYSPARAM_ERR_FULL = -3, - SYSPARAM_ERR_BADVALUE = -2, - SYSPARAM_ERR_NOINIT = -1, - SYSPARAM_OK = 0, - SYSPARAM_NOTFOUND = 1, - SYSPARAM_PARSEFAILED = 2, + SYSPARAM_OK = 0, ///< Success + SYSPARAM_NOTFOUND = 1, ///< Entry not found matching criteria + SYSPARAM_PARSEFAILED = 2, ///< Unable to parse retrieved value + SYSPARAM_ERR_NOINIT = -1, ///< sysparam_init() must be called first + SYSPARAM_ERR_BADVALUE = -2, ///< One or more arguments were invalid + SYSPARAM_ERR_FULL = -3, ///< No space left in sysparam area (or too many keys in use) + SYSPARAM_ERR_IO = -4, ///< I/O error reading/writing flash + SYSPARAM_ERR_CORRUPT = -5, ///< Sysparam region has bad/corrupted data + SYSPARAM_ERR_NOMEM = -6, ///< Unable to allocate memory } sysparam_status_t; - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_BADVALUE One or more arguments are invalid - * @retval SYSPARAM_ERR_FULL No space left in sysparam area - * (or too many keys in use) - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash - /** Structure used by sysparam_iter_next() to keep track of its current state * and return its results. This should be initialized by calling * sysparam_iter_start() and cleaned up afterward by calling @@ -62,16 +64,16 @@ typedef struct { * the normal initialization failed, or after calling sysparam_create_area() * to reformat the current area. * - * @param base_addr [in] The flash address which should contain the start of + * @param[in] base_addr The flash address which should contain the start of * the (already present) sysparam area * - * @retval SYSPARAM_OK Initialization successful. - * @retval SYSPARAM_NOTFOUND The specified address does not appear to - * contain a sysparam area. It may be necessary - * to call sysparam_create_area() to create one - * first. - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Initialization successful. + * @retval ::SYSPARAM_NOTFOUND The specified address does not appear to + * contain a sysparam area. It may be + * necessary to call sysparam_create_area() to + * create one first. + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_init(uint32_t base_addr); @@ -83,15 +85,15 @@ sysparam_status_t sysparam_init(uint32_t base_addr); * not overwrite it. Setting `force` to `true` will cause it to clobber any * existing data instead. * - * @param base_addr [in] The flash address at which it should start + * @param[in] base_addr The flash address at which it should start * (must be a multiple of the sector size) - * @param force [in] Proceed even if the space does not appear to be empty + * @param[in] force Proceed even if the space does not appear to be empty * - * @retval SYSPARAM_OK Area (re)created successfully. - * @retval SYSPARAM_NOTFOUND `force` was not specified, and the area at - * `base_addr` appears to have other data. No - * action taken. - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Area (re)created successfully. + * @retval ::SYSPARAM_NOTFOUND `force` was not specified, and the area at + * `base_addr` appears to have other data. No + * action taken. + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash * * Note: This routine can create a sysparam area in another location than the * one currently being used, but does not change which area is currently used @@ -109,26 +111,25 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); * contain any data (including zero bytes), so the `actual_length` parameter * should be used to determine the length of the data in the buffer. * - * Note: It is up to the caller to free() the returned buffer when done using - * it. + * It is up to the caller to free() the returned buffer when done using it. * - * @param key [in] Key name (zero-terminated string) - * @param destptr [out] Pointer to a location to hold the address of the - * returned data buffer - * @param actual_length [out] Pointer to a location to hold the length of the - * returned data buffer - * - * @retval SYSPARAM_OK Value successfully retrieved. - * @retval SYSPARAM_NOTFOUND Key/value not found. No buffer returned. - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash - * - * Note: If the result is anything other than SYSPARAM_OK, the value + * Note: If the status result is anything other than ::SYSPARAM_OK, the value * in `destptr` is not changed. This means it is possible to set a default * value before calling this function which will be left as-is if a sysparam * value could not be successfully read. + * + * @param[in] key Key name (zero-terminated string) + * @param[out] destptr Pointer to a location to hold the address of the + * returned data buffer + * @param[out] actual_length Pointer to a location to hold the length of the + * returned data buffer + * + * @retval ::SYSPARAM_OK Value successfully retrieved. + * @retval ::SYSPARAM_NOTFOUND Key/value not found. No buffer returned. + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length); @@ -146,75 +147,74 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * * must also be at least as large as the length of the key being requested. * If it is not, an error will be returned. * - * @param key [in] Key name (zero-terminated string) - * @param buffer [in] Pointer to a buffer to hold the returned value - * @param buffer_size [in] Length of the supplied buffer in bytes - * @param actual_length [out] pointer to a location to hold the actual length + * @param[in] key Key name (zero-terminated string) + * @param[in] buffer Pointer to a buffer to hold the returned value + * @param[in] buffer_size Length of the supplied buffer in bytes + * @param[out] actual_length pointer to a location to hold the actual length * of the data which was associated with the key * (may be NULL). * - * @retval SYSPARAM_OK Value successfully retrieved - * @retval SYSPARAM_NOTFOUND Key/value not found - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_NOMEM The supplied buffer is too small - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Value successfully retrieved + * @retval ::SYSPARAM_NOTFOUND Key/value not found + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_NOMEM The supplied buffer is too small + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); -/** Return the string value associated with a key +/** Get the string value associated with a key * * This routine can be used if you know that the value in a key will (or at * least should) be a string. It will return a zero-terminated char buffer * containing the value retrieved. * - * Note: It is up to the caller to free() the returned buffer when done using - * it. + * It is up to the caller to free() the returned buffer when done using it. * - * @param key [in] Key name (zero-terminated string) - * @param destptr [out] Pointer to a location to hold the address of the - * returned data buffer - * - * @retval SYSPARAM_OK Value successfully retrieved. - * @retval SYSPARAM_NOTFOUND Key/value not found. - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash - * - * Note: If the result is anything other than SYSPARAM_OK, the value + * Note: If the status result is anything other than ::SYSPARAM_OK, the value * in `destptr` is not changed. This means it is possible to set a default * value before calling this function which will be left as-is if a sysparam * value could not be successfully read. + * + * @param[in] key Key name (zero-terminated string) + * @param[out] destptr Pointer to a location to hold the address of the + * returned data buffer + * + * @retval ::SYSPARAM_OK Value successfully retrieved. + * @retval ::SYSPARAM_NOTFOUND Key/value not found. + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_get_string(const char *key, char **destptr); -/** Return the int32_t value associated with a key +/** Get the int32_t value associated with a key * * This routine can be used if you know that the value in a key will (or at * least should) be an integer value. It will parse the stored data as a * number (in standard decimal or "0x" hex notation) and return the result. * - * @param key [in] Key name (zero-terminated string) - * @param result [out] Pointer to a location to hold returned integer value - * - * @retval SYSPARAM_OK Value successfully retrieved. - * @retval SYSPARAM_NOTFOUND Key/value not found. - * @retval SYSPARAM_PARSEFAILED The retrieved value could not be parsed as an - * integer. - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash - * - * Note: If the result is anything other than SYSPARAM_OK, the value + * Note: If the status result is anything other than ::SYSPARAM_OK, the value * in `result` is not changed. This means it is possible to set a default * value before calling this function which will be left as-is if a sysparam * value could not be successfully read. + * + * @param[in] key Key name (zero-terminated string) + * @param[out] result Pointer to a location to hold returned integer value + * + * @retval ::SYSPARAM_OK Value successfully retrieved. + * @retval ::SYSPARAM_NOTFOUND Key/value not found. + * @retval ::SYSPARAM_PARSEFAILED The retrieved value could not be parsed as + * an integer. + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_get_int(const char *key, int32_t *result); -/** Return the boolean value associated with a key +/** Get the boolean value associated with a key * * This routine can be used if you know that the value in a key will (or at * least should) be a boolean setting. It will read the specified value as a @@ -224,22 +224,22 @@ sysparam_status_t sysparam_get_int(const char *key, int32_t *result); * * True: "yes", "y", "true", "t", "1" * * False: "no", "n", "false", "f", "0" * - * @param key [in] Key name (zero-terminated string) - * @param result [out] Pointer to a location to hold returned boolean value - * - * @retval SYSPARAM_OK Value successfully retrieved. - * @retval SYSPARAM_NOTFOUND Key/value not found. - * @retval SYSPARAM_PARSEFAILED The retrieved value could not be parsed as a - * boolean setting. - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash - * - * Note: If the result is anything other than SYSPARAM_OK, the value + * Note: If the status result is anything other than ::SYSPARAM_OK, the value * in `result` is not changed. This means it is possible to set a default * value before calling this function which will be left as-is if a sysparam * value could not be successfully read. + * + * @param[in] key Key name (zero-terminated string) + * @param[out] result Pointer to a location to hold returned boolean value + * + * @retval ::SYSPARAM_OK Value successfully retrieved. + * @retval ::SYSPARAM_NOTFOUND Key/value not found. + * @retval ::SYSPARAM_PARSEFAILED The retrieved value could not be parsed as a + * boolean setting. + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_get_bool(const char *key, bool *result); @@ -249,19 +249,19 @@ sysparam_status_t sysparam_get_bool(const char *key, bool *result); * is NULL or `value_len` is 0, this is treated as a request to delete any * current entry matching `key`. * - * @param key [in] Key name (zero-terminated string) - * @param value [in] Pointer to a buffer containing the value data - * @param value_len [in] Length of the data in the buffer + * @param[in] key Key name (zero-terminated string) + * @param[in] value Pointer to a buffer containing the value data + * @param[in] value_len Length of the data in the buffer * - * @retval SYSPARAM_OK Value successfully set. - * @retval SYSPARAM_ERR_NOINIT sysparam_init() must be called first - * @retval SYSPARAM_ERR_BADVALUE Either an empty key was provided or value_len - * is too large - * @retval SYSPARAM_ERR_FULL No space left in sysparam area - * (or too many keys in use) - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Value successfully set. + * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first + * @retval ::SYSPARAM_ERR_BADVALUE Either an empty key was provided or + * value_len is too large + * @retval ::SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len); @@ -270,17 +270,17 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ * Performs the same function as sysparam_set_data(), but accepts a * zero-terminated string value instead. * - * @param key [in] Key name (zero-terminated string) - * @param value [in] Value to set (zero-terminated string) + * @param[in] key Key name (zero-terminated string) + * @param[in] value Value to set (zero-terminated string) * - * @retval SYSPARAM_OK Value successfully set. - * @retval SYSPARAM_ERR_BADVALUE Either an empty key was provided or the - * length of `value` is too large - * @retval SYSPARAM_ERR_FULL No space left in sysparam area - * (or too many keys in use) - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Value successfully set. + * @retval ::SYSPARAM_ERR_BADVALUE Either an empty key was provided or the + * length of `value` is too large + * @retval ::SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_set_string(const char *key, const char *value); @@ -290,16 +290,16 @@ sysparam_status_t sysparam_set_string(const char *key, const char *value); * specified key. This does the inverse of the sysparam_get_int() * function. * - * @param key [in] Key name (zero-terminated string) - * @param value [in] Value to set + * @param[in] key Key name (zero-terminated string) + * @param[in] value Value to set * - * @retval SYSPARAM_OK Value successfully set. - * @retval SYSPARAM_ERR_BADVALUE An empty key was provided. - * @retval SYSPARAM_ERR_FULL No space left in sysparam area - * (or too many keys in use) - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Value successfully set. + * @retval ::SYSPARAM_ERR_BADVALUE An empty key was provided. + * @retval ::SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_set_int(const char *key, int32_t value); @@ -312,16 +312,16 @@ sysparam_status_t sysparam_set_int(const char *key, int32_t value); * Note that if the key already contains a value which parses to the same * boolean (true/false) value, it is left unchanged. * - * @param key [in] Key name (zero-terminated string) - * @param value [in] Value to set + * @param[in] key Key name (zero-terminated string) + * @param[in] value Value to set * - * @retval SYSPARAM_OK Value successfully set. - * @retval SYSPARAM_ERR_BADVALUE An empty key was provided. - * @retval SYSPARAM_ERR_FULL No space left in sysparam area - * (or too many keys in use) - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Value successfully set. + * @retval ::SYSPARAM_ERR_BADVALUE An empty key was provided. + * @retval ::SYSPARAM_ERR_FULL No space left in sysparam area + * (or too many keys in use) + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_set_bool(const char *key, bool value); @@ -335,10 +335,10 @@ sysparam_status_t sysparam_set_bool(const char *key, bool value); * NOTE: When done, you must call sysparam_iter_end() to free the resources * associated with `iter`, or you will leak memory. * - * @param iter [in] A pointer to a sysparam_iter_t structure to initialize + * @param[in] iter A pointer to a sysparam_iter_t structure to initialize * - * @retval SYSPARAM_OK Initialization successful - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_OK Initialization successful + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory */ sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter); @@ -353,12 +353,12 @@ sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter); * sysparam_iter_next() using the same `iter`. They should *not* be free()d * after use. * - * @param iter [in] The iterator structure to update + * @param[in] iter The iterator structure to update * - * @retval SYSPARAM_OK Next key/value retrieved - * @retval SYSPARAM_ERR_NOMEM Unable to allocate memory - * @retval SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data - * @retval SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Next key/value retrieved + * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory + * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter); From c772ea043d2552ca42d69eba4aa40df9a18f53ce Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Tue, 8 Mar 2016 23:13:15 -0800 Subject: [PATCH 07/11] Added a couple more debug statements --- core/include/sysparam.h | 10 +++++++--- core/sysparam.c | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index b4b2337..ce7edc0 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -1,18 +1,22 @@ #ifndef _SYSPARAM_H_ #define _SYSPARAM_H_ +#include + /** @file sysparam.h * * Read/write "system parameters" to persistent flash. * * System parameters are stored as key/value pairs. Keys are string values * between 1 and 255 characters long. Values can be any data up to 255 bytes - * in length (but are most commonly also strings). + * in length (but are most commonly also text strings). Up to 126 key/value + * pairs can be stored at a time. * + * Keys and values are stored in flash using a progressive list structure + * which allows space-efficient storage and minimizes flash erase cycles, + * improving write speed and increasing the lifespan of the flash memory. */ -#include - #ifndef SYSPARAM_REGION_SECTORS /** Number of (4K) sectors that make up a sysparam region. Total sysparam data * cannot be larger than this. Note that the full sysparam area is two diff --git a/core/sysparam.c b/core/sysparam.c index b664dbd..ced4ad8 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -577,6 +577,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ if (!value) value_len = 0; + debug(1, "updating value for '%s' (%d bytes)", key, value_len); if (value_len && ((intptr_t)value & 0x3)) { // The passed value isn't word-aligned. This will be a problem later // when calling `sdk_spi_flash_write`, so make a word-aligned copy. @@ -709,6 +710,8 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ _sysparam_info.end_addr = write_ctx.addr; } + debug(1, "new addr is 0x%08x (%d bytes remaining)", _sysparam_info.end_addr, _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4); + // Delete old value (if present) by setting it's id to 0x00 if (old_value_addr) { status = _delete_entry(old_value_addr); From c007c57be620c2d5324291b1549db5ace1aa4db2 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Tue, 15 Mar 2016 18:04:13 -0700 Subject: [PATCH 08/11] Fix potential memory leak if realloc() fails --- core/sysparam.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/core/sysparam.c b/core/sysparam.c index ced4ad8..90a6579 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -436,6 +436,7 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * sysparam_status_t status; size_t key_len = strlen(key); uint8_t *buffer; + uint8_t *newbuf; if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; @@ -450,10 +451,12 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * status = _find_entry(&ctx, ctx.entry.id | 0x80); if (status != SYSPARAM_OK) break; - buffer = realloc(buffer, ctx.entry.len + 1); - if (!buffer) { - return SYSPARAM_ERR_NOMEM; + newbuf = realloc(buffer, ctx.entry.len + 1); + if (!newbuf) { + status = SYSPARAM_ERR_NOMEM; + break; } + buffer = newbuf; status = _read_payload(&ctx, buffer, ctx.entry.len); if (status != SYSPARAM_OK) break; @@ -565,6 +568,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ sysparam_status_t status = SYSPARAM_OK; uint8_t key_len = strlen(key); uint8_t *buffer; + uint8_t *newbuf; size_t free_space; size_t needed_space; bool free_value = false; @@ -612,8 +616,12 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ if (ctx.entry.len == value_len) { // Are we trying to write the same value that's already there? if (value_len > key_len) { - buffer = realloc(buffer, value_len); - if (!buffer) return SYSPARAM_ERR_NOMEM; + newbuf = realloc(buffer, value_len); + if (!newbuf) { + status = SYSPARAM_ERR_NOMEM; + break; + } + buffer = newbuf; } status = _read_payload(&ctx, buffer, value_len); if (status == SYSPARAM_ERR_CORRUPT) { @@ -779,6 +787,7 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { struct sysparam_context *ctx = iter->ctx; struct sysparam_context value_ctx; size_t key_space; + char *newbuf; while (true) { status = _find_key(ctx, NULL, 0, buffer); @@ -792,11 +801,11 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { key_space = ROUND_TO_WORD_BOUNDARY(ctx->entry.len + 1); required_len = key_space + value_ctx.entry.len + 1; if (required_len > iter->bufsize) { - iter->key = realloc(iter->key, required_len); - if (!iter->key) { - iter->bufsize = 0; + newbuf = realloc(iter->key, required_len); + if (!newbuf) { return SYSPARAM_ERR_NOMEM; } + iter->key = newbuf; iter->bufsize = required_len; } From 20909a4da157e8273e328df251b701a4af30dca6 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Fri, 29 Apr 2016 09:49:05 -0700 Subject: [PATCH 09/11] Major sysparam overhaul --- core/include/sysparam.h | 65 ++- core/sysparam.c | 590 ++++++++++++++------- examples/sysparam_editor/Makefile | 4 +- examples/sysparam_editor/sysparam_editor.c | 122 ++++- 4 files changed, 547 insertions(+), 234 deletions(-) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index ce7edc0..9854f60 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -17,15 +17,6 @@ * improving write speed and increasing the lifespan of the flash memory. */ -#ifndef SYSPARAM_REGION_SECTORS -/** Number of (4K) sectors that make up a sysparam region. Total sysparam data - * cannot be larger than this. Note that the full sysparam area is two - * regions, so the actual amount of used flash space will be *twice* this - * amount. - */ -#define SYSPARAM_REGION_SECTORS 1 -#endif - /** Status codes returned by all sysparam functions * * Error codes (`SYSPARAM_ERR_*`) all have values less than zero, and can be @@ -54,6 +45,7 @@ typedef struct { uint8_t *value; size_t key_len; size_t value_len; + bool binary; size_t bufsize; struct sysparam_context *ctx; } sysparam_iter_t; @@ -68,8 +60,14 @@ typedef struct { * the normal initialization failed, or after calling sysparam_create_area() * to reformat the current area. * - * @param[in] base_addr The flash address which should contain the start of + * This routine will start at `base_addr` and scan all sectors up to + * `top_addr` looking for a valid sysparam area. If `top_addr` is zero (or + * equal to `base_addr`, then only the sector at `base_addr` will be checked. + * + * @param[in] base_addr The flash address to start looking for the start of * the (already present) sysparam area + * @param[in] top_addr The flash address to stop looking for the sysparam + * area * * @retval ::SYSPARAM_OK Initialization successful. * @retval ::SYSPARAM_NOTFOUND The specified address does not appear to @@ -79,7 +77,7 @@ typedef struct { * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ -sysparam_status_t sysparam_init(uint32_t base_addr); +sysparam_status_t sysparam_init(uint32_t base_addr, uint32_t top_addr); /** Create a new sysparam area in flash at the specified address. * @@ -89,15 +87,21 @@ sysparam_status_t sysparam_init(uint32_t base_addr); * not overwrite it. Setting `force` to `true` will cause it to clobber any * existing data instead. * - * @param[in] base_addr The flash address at which it should start - * (must be a multiple of the sector size) - * @param[in] force Proceed even if the space does not appear to be empty + * @param[in] base_addr The flash address at which it should start + * (must be a multiple of the sector size) + * @param[in] num_sectors The number of flash sectors to use for the sysparam + * area. This should be an even number >= 2. Note + * that the actual amount of useable parameter space + * will be roughly half this amount. + * @param[in] force Proceed even if the space does not appear to be empty * - * @retval ::SYSPARAM_OK Area (re)created successfully. - * @retval ::SYSPARAM_NOTFOUND `force` was not specified, and the area at - * `base_addr` appears to have other data. No - * action taken. - * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash + * @retval ::SYSPARAM_OK Area (re)created successfully. + * @retval ::SYSPARAM_NOTFOUND `force` was not specified, and the area at + * `base_addr` appears to have other data. No + * action taken. + * @retval ::SYSPARAM_ERR_BADVALUE The `num_sectors` value was not even (or + * was zero) + * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash * * Note: This routine can create a sysparam area in another location than the * one currently being used, but does not change which area is currently used @@ -106,7 +110,7 @@ sysparam_status_t sysparam_init(uint32_t base_addr); * sysparam_init() again afterward before you will be able to continue using * it. */ -sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); +sysparam_status_t sysparam_create_area(uint32_t base_addr, uint16_t num_sectors, bool force); /** Get the value associated with a key * @@ -126,7 +130,9 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); * @param[out] destptr Pointer to a location to hold the address of the * returned data buffer * @param[out] actual_length Pointer to a location to hold the length of the - * returned data buffer + * returned data buffer (may be NULL) + * @param[out] is_binary Pointer to a bool to hold whether the returned + * value is "binary" or not (may be NULL) * * @retval ::SYSPARAM_OK Value successfully retrieved. * @retval ::SYSPARAM_NOTFOUND Key/value not found. No buffer returned. @@ -135,7 +141,7 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force); * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ -sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length); +sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length, bool *is_binary); /** Get the value associate with a key (static buffers only) * @@ -157,6 +163,8 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * * @param[out] actual_length pointer to a location to hold the actual length * of the data which was associated with the key * (may be NULL). + * @param[out] is_binary Pointer to a bool to hold whether the returned + * value is "binary" or not (may be NULL) * * @retval ::SYSPARAM_OK Value successfully retrieved * @retval ::SYSPARAM_NOTFOUND Key/value not found @@ -165,7 +173,7 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ -sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length); +sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length, bool *is_binary); /** Get the string value associated with a key * @@ -186,6 +194,7 @@ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, siz * * @retval ::SYSPARAM_OK Value successfully retrieved. * @retval ::SYSPARAM_NOTFOUND Key/value not found. + * @retval ::SYSPARAM_PARSEFAILED The retrieved value was a binary value * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first * @retval ::SYSPARAM_ERR_NOMEM Unable to allocate memory * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data @@ -253,9 +262,17 @@ sysparam_status_t sysparam_get_bool(const char *key, bool *result); * is NULL or `value_len` is 0, this is treated as a request to delete any * current entry matching `key`. * + * If `binary` is true, the data will be considered binary (unprintable) data, + * and this will be annotated in the saved entry. This does not affect the + * saving or loading process in any way, but may be used by some applications + * to (for example) print binary data differently than text entries when + * printing parameter values. + * * @param[in] key Key name (zero-terminated string) * @param[in] value Pointer to a buffer containing the value data * @param[in] value_len Length of the data in the buffer + * @param[in] binary Whether the data should be considered "binary" + * (unprintable) data * * @retval ::SYSPARAM_OK Value successfully set. * @retval ::SYSPARAM_ERR_NOINIT sysparam_init() must be called first @@ -267,7 +284,7 @@ sysparam_status_t sysparam_get_bool(const char *key, bool *result); * @retval ::SYSPARAM_ERR_CORRUPT Sysparam region has bad/corrupted data * @retval ::SYSPARAM_ERR_IO I/O error reading/writing flash */ -sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len); +sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len, bool binary); /** Set a key's value from a string * diff --git a/core/sysparam.c b/core/sysparam.c index 90a6579..0aa2cb2 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -3,19 +3,14 @@ #include #include #include +#include //TODO: make this properly threadsafe //TODO: reduce stack usage -//FIXME: CRC calculations/checking -/* The "magic" values that indicate the start of a sysparam region in flash. - * Note that SYSPARAM_STALE_MAGIC is written over SYSPARAM_ACTIVE_MAGIC, so it - * must not contain any set bits which are not set in SYSPARAM_ACTIVE_MAGIC - * (that is, going from SYSPARAM_ACTIVE_MAGIC to SYSPARAM_STALE_MAGIC must only - * clear bits, not set them) +/* The "magic" value that indicates the start of a sysparam region in flash. */ -#define SYSPARAM_ACTIVE_MAGIC 0x70524f45 // "EORp" in little-endian -#define SYSPARAM_STALE_MAGIC 0x40524f45 // "EOR@" in little-endian +#define SYSPARAM_MAGIC 0x70524f45 // "EORp" in little-endian /* The size of the initial buffer created by sysparam_iter_start, etc, to hold * returned key-value pairs. Setting this too small may result in a lot of @@ -30,17 +25,41 @@ */ #define SCAN_BUFFER_SIZE 8 // words +/* The size of the temporary buffer used for reading back and verifying data + * written to flash. Making this larger will make the write-and-verify + * operation slightly faster, but will use more heap during writes + */ +#define VERIFY_BUF_SIZE 64 + /* Size of region/entry headers. These should not normally need tweaking (and * will probably require some code changes if they are tweaked). */ -#define REGION_HEADER_SIZE 4 // NOTE: Must be multiple of 4 +#define REGION_HEADER_SIZE 8 // NOTE: Must be multiple of 4 #define ENTRY_HEADER_SIZE 4 // NOTE: Must be multiple of 4 -/* Maximum value that can be used for a key_id. This is limited by the format - * to 0x7e (0x7f would produce a corresponding value ID of 0xff, which is - * invalid) +/* These are limited by the format to 0xffff, but could be set lower if desired */ -#define MAX_KEY_ID 0x7e +#define MAX_KEY_LEN 0xffff +#define MAX_VALUE_LEN 0xffff + +/* Maximum value that can be used for a key_id. This is limited by the format + * to 0xffe (0xfff indicates end/unwritten space) + */ +#define MAX_KEY_ID 0x0ffe + +#define REGION_FLAG_SECOND 0x8000 // First (0) or second (1) region +#define REGION_FLAG_ACTIVE 0x4000 // Stale (0) or active (1) region +#define REGION_MASK_SIZE 0x0fff // Region size in sectors + +#define ENTRY_FLAG_ALIVE 0x8000 // Deleted (0) or active (1) +#define ENTRY_FLAG_INVALID 0x4000 // Valid (0) or invalid (1) entry +#define ENTRY_FLAG_VALUE 0x2000 // Key (0) or value (1) +#define ENTRY_FLAG_BINARY 0x1000 // Text (0) or binary (1) data + +#define ENTRY_MASK_ID 0xfff + +#define ENTRY_ID_END 0xfff +#define ENTRY_ID_ANY 0x1000 #ifndef SYSPARAM_DEBUG #define SYSPARAM_DEBUG 0 @@ -60,19 +79,23 @@ /********************* Internal datatypes and structures *********************/ +struct region_header { + uint32_t magic; + uint16_t flags_size; + uint16_t reserved; +} __attribute__ ((packed)); + struct entry_header { - uint8_t prev_len; - uint8_t len; - uint8_t id; - uint8_t crc; + uint16_t idflags; + uint16_t len; } __attribute__ ((packed)); struct sysparam_context { uint32_t addr; struct entry_header entry; - uint64_t unused_keys[2]; + int unused_keys; size_t compactable; - uint8_t max_key_id; + uint16_t max_key_id; }; /*************************** Global variables/data ***************************/ @@ -82,26 +105,90 @@ static struct { uint32_t alt_base; uint32_t end_addr; size_t region_size; + bool force_compact; } _sysparam_info; /***************************** Internal routines *****************************/ +static inline IRAM sysparam_status_t _do_write(uint32_t addr, const void *data, size_t data_size) { + CHECK_FLASH_OP(sdk_spi_flash_write(addr, data, data_size)); + return SYSPARAM_OK; +} + +static inline IRAM sysparam_status_t _do_verify(uint32_t addr, const void *data, void *buffer, size_t len) { + CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, len)); + if (memcmp(data, buffer, len)) { + return SYSPARAM_ERR_IO; + } + return SYSPARAM_OK; +} + +/*FIXME: Eventually, this should probably be implemented down at the SPI flash library layer, where it can just compare bytes/words straight from the SPI hardware buffer instead of allocating a whole separate temp buffer, reading chunks into that, and then doing a memcmp.. */ +static IRAM sysparam_status_t _write_and_verify(uint32_t addr, const void *data, size_t data_size) { + int i; + size_t count; + sysparam_status_t status = SYSPARAM_OK; + uint8_t *verify_buf = malloc(VERIFY_BUF_SIZE); + + if (!verify_buf) return SYSPARAM_ERR_NOMEM; + do { + status = _do_write(addr, data, data_size); + if (status != SYSPARAM_OK) break; + for (i = 0; i < data_size; i += VERIFY_BUF_SIZE) { + count = min(data_size - i, VERIFY_BUF_SIZE); + status = _do_verify(addr + i, data + i, verify_buf, count); + if (status != SYSPARAM_OK) { + debug(1, "Flash write (@ 0x%08x) verify failed!", addr); + break; + } + } + } while (false); + free(verify_buf); + return status; +} + /** Erase the sectors of a region */ -static sysparam_status_t _format_region(uint32_t addr) { +static sysparam_status_t _format_region(uint32_t addr, uint16_t num_sectors) { uint16_t sector = addr / sdk_flashchip.sector_size; int i; - for (i = 0; i < SYSPARAM_REGION_SECTORS; i++) { + for (i = 0; i < num_sectors; i++) { CHECK_FLASH_OP(sdk_spi_flash_erase_sector(sector + i)); } return SYSPARAM_OK; } -/** Write the magic value at the beginning of a region */ -static inline sysparam_status_t _write_region_header(uint32_t addr, bool active) { - uint32_t magic = active ? SYSPARAM_ACTIVE_MAGIC : SYSPARAM_STALE_MAGIC; - debug(3, "write region header (0x%08x) @ 0x%08x", magic, addr); - CHECK_FLASH_OP(sdk_spi_flash_write(addr, &magic, 4)); +/** Write the magic data at the beginning of a region */ +static inline sysparam_status_t _write_region_header(uint32_t addr, uint32_t other, bool active) { + struct region_header header; + sysparam_status_t status; + int16_t num_sectors; + + header.magic = SYSPARAM_MAGIC; + if (addr < other) { + num_sectors = (other - addr) / sdk_flashchip.sector_size; + header.flags_size = num_sectors & REGION_MASK_SIZE; + } else { + num_sectors = (addr - other) / sdk_flashchip.sector_size; + header.flags_size = num_sectors & REGION_MASK_SIZE; + header.flags_size |= REGION_FLAG_SECOND; + } + if (active) { + header.flags_size |= REGION_FLAG_ACTIVE; + } + header.reserved = 0; + + debug(3, "write region header (0x%04x) @ 0x%08x", header.flags_size, addr); + status = _write_and_verify(addr, &header, REGION_HEADER_SIZE); + if (status != SYSPARAM_OK) { + // Uh oh.. Something failed, so we don't know whether what we wrote is + // actually in the flash or not. Try to zero it out to be sure and + // return an error. + debug(3, "zero region header @ 0x%08x", addr); + memset(&header, 0, REGION_HEADER_SIZE); + _write_and_verify(addr, &header, REGION_HEADER_SIZE); + return SYSPARAM_ERR_IO; + } return SYSPARAM_OK; } @@ -122,51 +209,79 @@ static sysparam_status_t init_write_context(struct sysparam_context *ctx) { /** Search through the region for an entry matching the specified id * - * @param match_id The id to match, or 0 to match any key, or 0xff to scan + * @param match_id The id to match, or 0 to match any key, or 0xfff to scan * to the end. */ -static sysparam_status_t _find_entry(struct sysparam_context *ctx, uint8_t match_id) { - uint8_t prev_len; +static sysparam_status_t _find_entry(struct sysparam_context *ctx, uint16_t match_id, bool find_value) { + uint16_t id; - while (ctx->addr + ENTRY_SIZE(ctx->entry.len) < _sysparam_info.end_addr) { - prev_len = ctx->entry.len; - ctx->addr += ENTRY_SIZE(ctx->entry.len); + while (true) { + if (ctx->addr == _sysparam_info.cur_base) { + ctx->addr += REGION_HEADER_SIZE; + } else { + uint32_t next_addr = ctx->addr + ENTRY_SIZE(ctx->entry.len); + if (next_addr > _sysparam_info.cur_base + _sysparam_info.region_size) { + // This entry has an obviously impossible length, so we need to + // stop reading here. + // We can report this as the end of the valid entries, but then + // any future writes (to the end) will write over + // previously-written data and result in garbage. The best + // workaround is to make sure that the next write operation + // will always start with a compaction, which will leave off + // the invalid data at the end and fix the issue going forward. + debug(1, "Encountered entry with invalid length (0x%04x) @ 0x%08x (region end is 0x%08x). Truncating entries.", ctx->entry.len, ctx->addr, _sysparam_info.end_addr); + _sysparam_info.force_compact = true; + break; + } + ctx->addr = next_addr; + if (ctx->addr == _sysparam_info.cur_base + _sysparam_info.region_size) { + // This is the last entry in the available space, but it + // exactly fits. Stop reading here. + break; + } + } debug(3, "read entry header @ 0x%08x", ctx->addr); CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr, &ctx->entry, ENTRY_HEADER_SIZE)); - if (ctx->entry.prev_len != prev_len) { - // Uh oh.. This should match the entry.len field from the - // previous entry. If it doesn't, it means that field may have - // been corrupted and we don't even know if we're in the right - // place anymore. We have to bail out. - debug(1, "prev_len mismatch at 0x%08x (%d != %d)", ctx->addr, ctx->entry.prev_len, prev_len); - ctx->addr = _sysparam_info.end_addr; - return SYSPARAM_ERR_CORRUPT; + debug(3, " idflags = 0x%04x", ctx->entry.idflags); + if (ctx->entry.idflags == 0xffff) { + // 0xffff is never a valid id field, so this means we've hit the + // end and are looking at unwritten flash space from here on. + break; } - if (ctx->entry.id) { - if (!(ctx->entry.id & 0x80)) { - // Key definition - ctx->max_key_id = ctx->entry.id; - ctx->unused_keys[ctx->entry.id >> 6] |= (1 << (ctx->entry.id & 0x3f)); - if (!match_id) { - // We're looking for any key, so make this a matching key. - match_id = ctx->entry.id; + id = ctx->entry.idflags & ENTRY_MASK_ID; + if ((ctx->entry.idflags & (ENTRY_FLAG_ALIVE | ENTRY_FLAG_INVALID)) == ENTRY_FLAG_ALIVE) { + debug(3, " entry is alive and valid"); + if (!(ctx->entry.idflags & ENTRY_FLAG_VALUE)) { + debug(3, " entry is a key"); + ctx->max_key_id = id; + ctx->unused_keys++; + if (!find_value) { + if ((id == match_id) || (match_id == ENTRY_ID_ANY)) { + return SYSPARAM_OK; + } } } else { - // Value entry - ctx->unused_keys[(ctx->entry.id >> 6) & 1] &= ~(1 << (ctx->entry.id & 0x3f)); - } - if (ctx->entry.id == match_id) { - return SYSPARAM_OK; + debug(3, " entry is a value"); + ctx->unused_keys--; + if (find_value) { + if ((id == match_id) || (match_id == ENTRY_ID_ANY)) { + return SYSPARAM_OK; + } + } } + debug(3, " (not a match)"); } else { - // Deleted entry + debug(3, " entry is deleted or invalid"); ctx->compactable += ENTRY_SIZE(ctx->entry.len); } } + if (match_id == ENTRY_ID_END) { + return SYSPARAM_OK; + } ctx->entry.len = 0; - ctx->entry.id = 0; + ctx->entry.idflags = 0; return SYSPARAM_NOTFOUND; } @@ -174,18 +289,19 @@ static sysparam_status_t _find_entry(struct sysparam_context *ctx, uint8_t match static inline sysparam_status_t _read_payload(struct sysparam_context *ctx, uint8_t *buffer, size_t buffer_size) { debug(3, "read payload (%d) @ 0x%08x", min(buffer_size, ctx->entry.len), ctx->addr); CHECK_FLASH_OP(sdk_spi_flash_read(ctx->addr + ENTRY_HEADER_SIZE, buffer, min(buffer_size, ctx->entry.len))); - //FIXME: check crc return SYSPARAM_OK; } /** Find the entry corresponding to the specified key name */ -static sysparam_status_t _find_key(struct sysparam_context *ctx, const char *key, uint8_t key_len, uint8_t *buffer) { +static sysparam_status_t _find_key(struct sysparam_context *ctx, const char *key, uint16_t key_len, uint8_t *buffer) { sysparam_status_t status; + debug(3, "find key: %s", key ? key : "(null)"); while (true) { // Find the next key entry - status = _find_entry(ctx, 0); + status = _find_entry(ctx, ENTRY_ID_ANY, false); if (status != SYSPARAM_OK) return status; + debug(3, "found a key entry @ 0x%08x", ctx->addr); if (!key) { // We're looking for the next (any) key, so we're done. break; @@ -197,43 +313,70 @@ static sysparam_status_t _find_key(struct sysparam_context *ctx, const char *key // We have a match break; } + debug(3, "entry payload does not match"); + } else { + debug(3, "key length (%d) does not match (%d)", ctx->entry.len, key_len); } } + debug(3, "key match @ 0x%08x (idflags = 0x%04x)", ctx->addr, ctx->entry.idflags); return SYSPARAM_OK; } +/** Find the value entry matching the id field from a particular key */ +static inline sysparam_status_t _find_value(struct sysparam_context *ctx, uint16_t id_field) { + debug(3, "find value: 0x%04x", id_field); + return _find_entry(ctx, id_field & ENTRY_MASK_ID, true); +} + /** Write an entry at the specified address */ -static inline sysparam_status_t _write_entry(uint32_t addr, uint8_t id, const uint8_t *payload, uint8_t len, uint8_t prev_len) { +static inline sysparam_status_t _write_entry(uint32_t addr, uint16_t id, const uint8_t *payload, uint16_t len) { struct entry_header entry; + sysparam_status_t status; debug(2, "Writing entry 0x%02x @ 0x%08x", id, addr); - entry.prev_len = prev_len; + entry.idflags = id | ENTRY_FLAG_ALIVE | ENTRY_FLAG_INVALID; entry.len = len; - entry.id = id; - entry.crc = 0; //FIXME: calculate crc - debug(3, "write entry header @ 0x%08x", addr); - CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); + debug(3, "write initial entry header @ 0x%08x", addr); + status = _write_and_verify(addr, &entry, ENTRY_HEADER_SIZE); + if (status == SYSPARAM_ERR_IO) { + // Uh-oh.. Either the flash call failed in some way or we didn't get + // back what we wrote. This could be a problem because depending on + // how it went wrong it could screw up all reads/writes from this point + // forward. Try to salvage the on-flash structure by overwriting the + // failed header with all zeros, which (if successful) will be + // interpreted on later reads as a deleted empty-payload entry (and it + // will just skip to the next spot). + memset(&entry, 0, ENTRY_HEADER_SIZE); + debug(3, "zeroing entry header @ 0x%08x", addr); + status = _write_and_verify(addr, &entry, ENTRY_HEADER_SIZE); + if (status != SYSPARAM_OK) return status; + + // Make sure future writes skip past this zeroed bit + if (_sysparam_info.end_addr == addr) { + _sysparam_info.end_addr += ENTRY_HEADER_SIZE; + } + // We could just skip to the next space and try again, but + // unfortunately now we can't be sure there's enough space remaining to + // fit the entry, so we just have to fail this operation. Hopefully, + // at least, future requests will still succeed, though. + status = SYSPARAM_ERR_IO; + } + if (status != SYSPARAM_OK) return status; + + // If we've gotten this far, we've committed to writing the full entry. + if (_sysparam_info.end_addr == addr) { + _sysparam_info.end_addr += ENTRY_SIZE(len); + } debug(3, "write payload (%d) @ 0x%08x", len, addr + ENTRY_HEADER_SIZE); - CHECK_FLASH_OP(sdk_spi_flash_write(addr + ENTRY_HEADER_SIZE, payload, len)); + status = _write_and_verify(addr + ENTRY_HEADER_SIZE, payload, len); + if (status != SYSPARAM_OK) return status; - return SYSPARAM_OK; -} + debug(3, "set entry valid @ 0x%08x", addr); + entry.idflags &= ~ENTRY_FLAG_INVALID; + status = _write_and_verify(addr, &entry, ENTRY_HEADER_SIZE); -/** Write the "tail" entry on the end of the region - * (this entry just contains the `prev_len` field with all others set to 0xff) - */ -static inline sysparam_status_t _write_entry_tail(uint32_t addr, uint8_t prev_len) { - struct entry_header entry; - - entry.prev_len = prev_len; - entry.len = 0xff; - entry.id = 0xff; - entry.crc = 0xff; - debug(3, "write entry tail @ 0x%08x", addr); - CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); - - return SYSPARAM_OK; + return status; } /** Mark an entry as "deleted" so it won't be considered in future reads */ @@ -244,7 +387,7 @@ static inline sysparam_status_t _delete_entry(uint32_t addr) { debug(3, "read entry header @ 0x%08x", addr); CHECK_FLASH_OP(sdk_spi_flash_read(addr, &entry, ENTRY_HEADER_SIZE)); // Set the ID to zero to mark it as "deleted" - entry.id = 0x00; + entry.idflags &= ~ENTRY_FLAG_ALIVE; debug(3, "write entry header @ 0x%08x", addr); CHECK_FLASH_OP(sdk_spi_flash_write(addr, &entry, ENTRY_HEADER_SIZE)); @@ -263,16 +406,17 @@ static inline sysparam_status_t _delete_entry(uint32_t addr) { * automatically update *key_id to contain the ID of this key in the new * compacted result as well. */ -static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t *key_id) { +static sysparam_status_t _compact_params(struct sysparam_context *ctx, int *key_id) { uint32_t new_base = _sysparam_info.alt_base; sysparam_status_t status; uint32_t addr = new_base + REGION_HEADER_SIZE; - uint8_t current_key_id = 0; + uint16_t current_key_id = 0; sysparam_iter_t iter; - uint8_t prev_len = 0; + uint16_t binary_flag; + uint16_t num_sectors = _sysparam_info.region_size / sdk_flashchip.sector_size; - debug(1, "compacting region (current size %d, expect to recover %d%s bytes)...", _sysparam_info.end_addr - _sysparam_info.cur_base, ctx->compactable, (ctx->unused_keys[0] || ctx->unused_keys[1]) ? "+ (unused keys present)" : ""); - status = _format_region(new_base); + debug(1, "compacting region (current size %d, expect to recover %d%s bytes)...", _sysparam_info.end_addr - _sysparam_info.cur_base, ctx->compactable, (ctx->unused_keys > 0) ? "+ (unused keys present)" : ""); + status = _format_region(new_base, num_sectors); if (status < 0) return status; status = sysparam_iter_start(&iter); if (status < 0) return status; @@ -285,12 +429,11 @@ static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t * // Write the key to the new region debug(2, "writing %d key @ 0x%08x", current_key_id, addr); - status = _write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len, prev_len); + status = _write_entry(addr, current_key_id, (uint8_t *)iter.key, iter.key_len); if (status < 0) break; - prev_len = iter.key_len; addr += ENTRY_SIZE(iter.key_len); - if (iter.ctx->entry.id == *key_id) { + if ((iter.ctx->entry.idflags & ENTRY_MASK_ID) == *key_id) { // Update key_id to have the correct id for the compacted result *key_id = current_key_id; // Don't copy the old value, since we'll just be deleting it @@ -300,17 +443,13 @@ static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t * // Copy the value to the new region debug(2, "writing %d value @ 0x%08x", current_key_id, addr); - status = _write_entry(addr, current_key_id | 0x80, iter.value, iter.value_len, prev_len); + binary_flag = iter.binary ? ENTRY_FLAG_BINARY : 0; + status = _write_entry(addr, current_key_id | ENTRY_FLAG_VALUE | binary_flag, iter.value, iter.value_len); if (status < 0) break; - prev_len = iter.value_len; addr += ENTRY_SIZE(iter.value_len); } sysparam_iter_end(&iter); - if (status >= 0) { - status = _write_entry_tail(addr, prev_len); - } - // If we broke out with an error, return the error instead of continuing. if (status < 0) { debug(1, "error encountered during compacting (%d)", status); @@ -318,19 +457,19 @@ static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t * } // Switch to officially using the new region. - status = _write_region_header(new_base, true); + status = _write_region_header(new_base, _sysparam_info.cur_base, true); if (status < 0) return status; - status = _write_region_header(_sysparam_info.cur_base, false); + status = _write_region_header(_sysparam_info.cur_base, new_base, false); if (status < 0) return status; _sysparam_info.alt_base = _sysparam_info.cur_base; _sysparam_info.cur_base = new_base; _sysparam_info.end_addr = addr; + _sysparam_info.force_compact = false; // Fix up ctx so it doesn't point to invalid stuff memset(ctx, 0, sizeof(*ctx)); ctx->addr = addr; - ctx->entry.prev_len = prev_len; ctx->max_key_id = current_key_id; debug(1, "done compacting (current size %d)", _sysparam_info.end_addr - _sysparam_info.cur_base); @@ -340,42 +479,82 @@ static sysparam_status_t _compact_params(struct sysparam_context *ctx, uint8_t * /***************************** Public Functions ******************************/ -sysparam_status_t sysparam_init(uint32_t base_addr) { +sysparam_status_t sysparam_init(uint32_t base_addr, uint32_t top_addr) { sysparam_status_t status; - uint32_t magic0, magic1; + uint32_t addr0, addr1; + struct region_header header0, header1; struct sysparam_context ctx; + uint16_t num_sectors; - _sysparam_info.region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; - // First, see if we can find an existing one. - debug(3, "read magic @ 0x%08x", base_addr); - CHECK_FLASH_OP(sdk_spi_flash_read(base_addr, &magic0, 4)); - debug(3, "read magic @ 0x%08x", base_addr + _sysparam_info.region_size); - CHECK_FLASH_OP(sdk_spi_flash_read(base_addr + _sysparam_info.region_size, &magic1, 4)); - if (magic0 == SYSPARAM_ACTIVE_MAGIC && magic1 == SYSPARAM_STALE_MAGIC) { - // Sysparam area found, first region is active - _sysparam_info.cur_base = base_addr; - _sysparam_info.alt_base = base_addr + _sysparam_info.region_size; - } else if (magic0 == SYSPARAM_STALE_MAGIC && magic1 == SYSPARAM_ACTIVE_MAGIC) { - // Sysparam area found, second region is active - _sysparam_info.cur_base = base_addr + _sysparam_info.region_size; - _sysparam_info.alt_base = base_addr; - } else if (magic0 == SYSPARAM_ACTIVE_MAGIC && magic1 == SYSPARAM_ACTIVE_MAGIC) { - // Both regions are marked as active. Not sure which to use. - // This can theoretically happen if something goes wrong at exactly the - // wrong time during compacting. - return SYSPARAM_ERR_CORRUPT; - } else if (magic0 == SYSPARAM_STALE_MAGIC && magic1 == SYSPARAM_STALE_MAGIC) { - // Both regions are marked as inactive. This shouldn't ever happen. - return SYSPARAM_ERR_CORRUPT; - } else { - // Looks like there's something else at that location entirely. + // Make sure we're starting at the beginning of the sector + base_addr -= (base_addr % sdk_flashchip.sector_size); + + if (!top_addr || top_addr == base_addr) { + // Only scan the specified sector, nowhere else. + top_addr = base_addr + sdk_flashchip.sector_size; + } + for (addr0 = base_addr; addr0 < top_addr; addr0 += sdk_flashchip.sector_size) { + CHECK_FLASH_OP(sdk_spi_flash_read(addr0, &header0, REGION_HEADER_SIZE)); + if (header0.magic == SYSPARAM_MAGIC) { + // Found a starting point... + break; + } + } + if (addr0 >= top_addr) { return SYSPARAM_NOTFOUND; } + // We've found a valid header at addr0. Now find the other half of the sysparam area. + num_sectors = header0.flags_size & REGION_MASK_SIZE; + + if (header0.flags_size & REGION_FLAG_SECOND) { + addr1 = addr0 - num_sectors * sdk_flashchip.sector_size; + } else { + addr1 = addr0 + num_sectors * sdk_flashchip.sector_size; + } + CHECK_FLASH_OP(sdk_spi_flash_read(addr1, &header1, REGION_HEADER_SIZE)); + + if (header1.magic == SYSPARAM_MAGIC) { + // Yay! Found the other one. Sanity-check it.. + if ((header0.flags_size & REGION_FLAG_SECOND) == (header1.flags_size & REGION_FLAG_SECOND)) { + // Hmm.. they both say they're the same region. That can't be right... + debug(1, "Found region headers @ 0x%08x and 0x%08x, but both claim to be the same region.", addr0, addr1); + return SYSPARAM_ERR_CORRUPT; + } + } else { + // Didn't find a valid header at the alternate location (which probably means something clobbered it or something went wrong at a critical point when rewriting it. Is the one we did find the active or stale one? + if (header0.flags_size & REGION_FLAG_ACTIVE) { + // Found the active one. We can work with this. Try to recreate the missing stale region... + debug(2, "Found active region header @ 0x%08x but no stale region @ 0x%08x. Trying to recreate stale region.", addr0, addr1); + status = _format_region(addr1, num_sectors); + if (status != SYSPARAM_OK) return status; + status = _write_region_header(addr1, addr0, false); + if (status != SYSPARAM_OK) return status; + } else { + // Found the stale one. We have no idea how old it is, so we shouldn't use it without some sort of confirmation/recovery. We'll have to bail for now. + debug(1, "Found stale-region header @ 0x%08x, but no active region.", addr0); + return SYSPARAM_ERR_CORRUPT; + } + } + // At this point we have confirmed valid regions at addr0 and addr1. + + _sysparam_info.region_size = num_sectors * sdk_flashchip.sector_size; + if (header0.flags_size & REGION_FLAG_ACTIVE) { + _sysparam_info.cur_base = addr0; + _sysparam_info.alt_base = addr1; + debug(3, "Active region @ 0x%08x (0x%04x). Stale region @ 0x%08x (0x%04x).", addr0, header0.flags_size, addr1, header1.flags_size); + + } else { + _sysparam_info.cur_base = addr1; + _sysparam_info.alt_base = addr0; + debug(3, "Active region @ 0x%08x (0x%04x). Stale region @ 0x%08x (0x%04x).", addr1, header1.flags_size, addr0, header0.flags_size); + } + // Find the actual end _sysparam_info.end_addr = _sysparam_info.cur_base + _sysparam_info.region_size; + _sysparam_info.force_compact = false; _init_context(&ctx); - status = _find_entry(&ctx, 0xff); + status = _find_entry(&ctx, ENTRY_ID_END, false); if (status < 0) { _sysparam_info.cur_base = 0; _sysparam_info.alt_base = 0; @@ -389,17 +568,24 @@ sysparam_status_t sysparam_init(uint32_t base_addr) { return SYSPARAM_OK; } -sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force) { +sysparam_status_t sysparam_create_area(uint32_t base_addr, uint16_t num_sectors, bool force) { + size_t region_size; sysparam_status_t status; uint32_t buffer[SCAN_BUFFER_SIZE]; uint32_t addr; int i; - size_t region_size = SYSPARAM_REGION_SECTORS * sdk_flashchip.sector_size; + + // Convert "number of sectors for area" into "number of sectors per region" + if (num_sectors < 1 || (num_sectors & 1)) { + return SYSPARAM_ERR_BADVALUE; + } + num_sectors >>= 1; + region_size = num_sectors * sdk_flashchip.sector_size; if (!force) { // First, scan through the area and make sure it's actually empty and // we're not going to be clobbering something else important. - for (addr = base_addr; addr < base_addr + SYSPARAM_REGION_SECTORS * 2 * sdk_flashchip.sector_size; addr += SCAN_BUFFER_SIZE) { + for (addr = base_addr; addr < base_addr + region_size * 2; addr += SCAN_BUFFER_SIZE) { debug(3, "read %d words @ 0x%08x", SCAN_BUFFER_SIZE, addr); CHECK_FLASH_OP(sdk_spi_flash_read(addr, buffer, SCAN_BUFFER_SIZE * 4)); for (i = 0; i < SCAN_BUFFER_SIZE; i++) { @@ -417,21 +603,19 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, bool force) { // `sysparam_init()` afterwards. memset(&_sysparam_info, 0, sizeof(_sysparam_info)); } - status = _format_region(base_addr); + status = _format_region(base_addr, num_sectors); if (status < 0) return status; - status = _format_region(base_addr + region_size); + status = _format_region(base_addr + region_size, num_sectors); if (status < 0) return status; - status = _write_entry_tail(base_addr + REGION_HEADER_SIZE, 0); + status = _write_region_header(base_addr, base_addr + region_size, true); if (status < 0) return status; - status = _write_region_header(base_addr + region_size, false); - if (status < 0) return status; - status = _write_region_header(base_addr, true); + status = _write_region_header(base_addr + region_size, base_addr, false); if (status < 0) return status; return SYSPARAM_OK; } -sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length) { +sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length, bool *is_binary) { struct sysparam_context ctx; sysparam_status_t status; size_t key_len = strlen(key); @@ -448,7 +632,7 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * if (status != SYSPARAM_OK) break; // Find the associated value - status = _find_entry(&ctx, ctx.entry.id | 0x80); + status = _find_value(&ctx, ctx.entry.idflags); if (status != SYSPARAM_OK) break; newbuf = realloc(buffer, ctx.entry.len + 1); @@ -467,6 +651,7 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * *destptr = buffer; if (actual_length) *actual_length = ctx.entry.len; + if (is_binary) *is_binary = (bool)(ctx.entry.idflags & ENTRY_FLAG_BINARY); return SYSPARAM_OK; } while (false); @@ -475,7 +660,7 @@ sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t * return status; } -sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length) { +sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, size_t buffer_size, size_t *actual_length, bool *is_binary) { struct sysparam_context ctx; sysparam_status_t status = SYSPARAM_OK; size_t key_len = strlen(key); @@ -491,19 +676,33 @@ sysparam_status_t sysparam_get_data_static(const char *key, uint8_t *buffer, siz _init_context(&ctx); status = _find_key(&ctx, key, key_len, buffer); if (status != SYSPARAM_OK) return status; - status = _find_entry(&ctx, ctx.entry.id | 0x80); + status = _find_value(&ctx, ctx.entry.idflags); if (status != SYSPARAM_OK) return status; status = _read_payload(&ctx, buffer, buffer_size); if (status != SYSPARAM_OK) return status; if (actual_length) *actual_length = ctx.entry.len; + if (is_binary) *is_binary = (bool)(ctx.entry.idflags & ENTRY_FLAG_BINARY); return SYSPARAM_OK; } sysparam_status_t sysparam_get_string(const char *key, char **destptr) { + bool is_binary; + sysparam_status_t status; + uint8_t *buf; + + status = sysparam_get_data(key, &buf, NULL, &is_binary); + if (status != SYSPARAM_OK) return status; + if (is_binary) { + // Value was saved as binary data, which means we shouldn't try to + // interpret it as a string. + free(buf); + return SYSPARAM_PARSEFAILED; + } // `sysparam_get_data` will zero-terminate the result as a matter of course, // so no need to do that here. - return sysparam_get_data(key, (uint8_t **)destptr, NULL); + *destptr = (char *)buf; + return SYSPARAM_OK; } sysparam_status_t sysparam_get_int(const char *key, int32_t *result) { @@ -528,29 +727,23 @@ sysparam_status_t sysparam_get_int(const char *key, int32_t *result) { sysparam_status_t sysparam_get_bool(const char *key, bool *result) { char *buffer; - int i; sysparam_status_t status; status = sysparam_get_string(key, &buffer); if (status != SYSPARAM_OK) return status; do { - for (i = 0; buffer[i]; i++) { - // Quick-and-dirty tolower(). Not perfect, but works for our - // purposes, and avoids needing to pull in additional libc stuff. - if (buffer[i] >= 0x41) buffer[i] |= 0x20; - } - if (!strcmp(buffer, "y") || - !strcmp(buffer, "yes") || - !strcmp(buffer, "t") || - !strcmp(buffer, "true") || + if (!strcasecmp(buffer, "y") || + !strcasecmp(buffer, "yes") || + !strcasecmp(buffer, "t") || + !strcasecmp(buffer, "true") || !strcmp(buffer, "1")) { *result = true; break; } - if (!strcmp(buffer, "n") || - !strcmp(buffer, "no") || - !strcmp(buffer, "f") || - !strcmp(buffer, "false") || + if (!strcasecmp(buffer, "n") || + !strcasecmp(buffer, "no") || + !strcasecmp(buffer, "f") || + !strcasecmp(buffer, "false") || !strcmp(buffer, "0")) { *result = false; break; @@ -562,22 +755,24 @@ sysparam_status_t sysparam_get_bool(const char *key, bool *result) { return status; } -sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len) { +sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_t value_len, bool is_binary) { struct sysparam_context ctx; struct sysparam_context write_ctx; sysparam_status_t status = SYSPARAM_OK; - uint8_t key_len = strlen(key); + uint16_t key_len = strlen(key); uint8_t *buffer; uint8_t *newbuf; size_t free_space; size_t needed_space; bool free_value = false; - uint8_t key_id = 0; + int key_id = -1; uint32_t old_value_addr = 0; + uint16_t binary_flag; if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; if (!key_len) return SYSPARAM_ERR_BADVALUE; - if (value_len > 0xff) return SYSPARAM_ERR_BADVALUE; + if (key_len > MAX_KEY_LEN) return SYSPARAM_ERR_BADVALUE; + if (value_len > MAX_VALUE_LEN) return SYSPARAM_ERR_BADVALUE; if (!value) value_len = 0; @@ -603,17 +798,19 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ status = _find_key(&ctx, key, key_len, buffer); if (status == SYSPARAM_OK) { // Key already exists, see if there's a current value. - key_id = ctx.entry.id; - status = _find_entry(&ctx, key_id | 0x80); + key_id = ctx.entry.idflags & ENTRY_MASK_ID; + status = _find_value(&ctx, key_id); if (status == SYSPARAM_OK) { old_value_addr = ctx.addr; } } if (status < 0) break; + binary_flag = is_binary ? ENTRY_FLAG_BINARY : 0; + if (value_len) { if (old_value_addr) { - if (ctx.entry.len == value_len) { + if ((ctx.entry.idflags & ENTRY_FLAG_BINARY) == binary_flag && ctx.entry.len == value_len) { // Are we trying to write the same value that's already there? if (value_len > key_len) { newbuf = realloc(buffer, value_len); @@ -624,12 +821,8 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ buffer = newbuf; } status = _read_payload(&ctx, buffer, value_len); - if (status == SYSPARAM_ERR_CORRUPT) { - // If the CRC check failed, don't worry about it. We're - // going to be deleting this entry anyway. - } else if (status < 0) { - break; - } else if (!memcmp(buffer, value, value_len)) { + if (status < 0) break; + if (!memcmp(buffer, value, value_len)) { // Yup, it's a match! No need to do anything further, // just leave the current value as-is. status = SYSPARAM_OK; @@ -645,9 +838,9 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ // Append new value to the end, but first make sure we have enough // space. - free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4; + free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr; needed_space = ENTRY_SIZE(value_len); - if (!key_id) { + if (key_id < 0) { // We did not find a previous key entry matching this key. We // will need to add a key entry as well. key_len = strlen(key); @@ -657,13 +850,13 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ // Can we compact things? // First, scan all remaining entries up to the end so we can // get a reasonably accurate "compactable" reading. - _find_entry(&ctx, 0xff); + _find_entry(&ctx, ENTRY_ID_END, false); if (needed_space <= free_space + ctx.compactable) { // We should be able to get enough space by compacting. status = _compact_params(&ctx, &key_id); if (status < 0) break; old_value_addr = 0; - } else if (ctx.unused_keys[0] || ctx.unused_keys[1]) { + } else if (ctx.unused_keys > 0) { // Compacting will gain more space than expected, because // there are some keys that can be omitted too, but we // don't know exactly how much that will gain, so all we @@ -672,7 +865,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ if (status < 0) break; old_value_addr = 0; } - free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4; + free_space = _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr; } if (needed_space > free_space) { // Nothing we can do here.. We're full. @@ -683,17 +876,14 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ break; } - init_write_context(&write_ctx); - - if (!key_id) { + if (key_id < 0) { // We need to write a key entry for a new key. // If we didn't find the key, then we already know _find_entry // has gone through the entire contents, and thus // ctx.max_key_id has the largest key_id found in the whole // region. - key_id = ctx.max_key_id + 1; - if (key_id > MAX_KEY_ID) { - if (ctx.unused_keys[0] || ctx.unused_keys[1]) { + if (ctx.max_key_id >= MAX_KEY_ID) { + if (ctx.unused_keys > 0) { status = _compact_params(&ctx, &key_id); if (status < 0) break; old_value_addr = 0; @@ -703,28 +893,40 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ break; } } - status = _write_entry(write_ctx.addr, key_id, (uint8_t *)key, key_len, write_ctx.entry.prev_len); + } + + if (_sysparam_info.force_compact) { + // We didn't need to compact above, but due to previously + // detected inconsistencies, we should compact anyway before + // writing anything new, so do that. + status = _compact_params(&ctx, &key_id); + if (status < 0) break; + } + + init_write_context(&write_ctx); + + if (key_id < 0) { + // Write a new key entry + key_id = ctx.max_key_id + 1; + status = _write_entry(write_ctx.addr, key_id, (uint8_t *)key, key_len); if (status < 0) break; write_ctx.addr += ENTRY_SIZE(key_len); - write_ctx.entry.prev_len = key_len; } // Write new value - status = _write_entry(write_ctx.addr, key_id | 0x80, value, value_len, write_ctx.entry.prev_len); + status = _write_entry(write_ctx.addr, key_id | ENTRY_FLAG_VALUE | binary_flag, value, value_len); if (status < 0) break; write_ctx.addr += ENTRY_SIZE(value_len); - status = _write_entry_tail(write_ctx.addr, value_len); - if (status < 0) break; _sysparam_info.end_addr = write_ctx.addr; } - debug(1, "new addr is 0x%08x (%d bytes remaining)", _sysparam_info.end_addr, _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr - 4); - - // Delete old value (if present) by setting it's id to 0x00 + // Delete old value (if present) by clearing its "alive" flag if (old_value_addr) { status = _delete_entry(old_value_addr); if (status < 0) break; } + + debug(1, "New addr is 0x%08x (%d bytes remaining)", _sysparam_info.end_addr, _sysparam_info.cur_base + _sysparam_info.region_size - _sysparam_info.end_addr); } while (false); if (free_value) free((void *)value); @@ -733,7 +935,7 @@ sysparam_status_t sysparam_set_data(const char *key, const uint8_t *value, size_ } sysparam_status_t sysparam_set_string(const char *key, const char *value) { - return sysparam_set_data(key, (const uint8_t *)value, strlen(value)); + return sysparam_set_data(key, (const uint8_t *)value, strlen(value), false); } sysparam_status_t sysparam_set_int(const char *key, int32_t value) { @@ -741,7 +943,7 @@ sysparam_status_t sysparam_set_int(const char *key, int32_t value) { int len; len = snprintf((char *)buffer, 12, "%d", value); - return sysparam_set_data(key, buffer, len); + return sysparam_set_data(key, buffer, len, false); } sysparam_status_t sysparam_set_bool(const char *key, bool value) { @@ -755,7 +957,7 @@ sysparam_status_t sysparam_set_bool(const char *key, bool value) { } buf[0] = value ? 'y' : 'n'; - return sysparam_set_data(key, buf, 1); + return sysparam_set_data(key, buf, 1, false); } sysparam_status_t sysparam_iter_start(sysparam_iter_t *iter) { @@ -794,7 +996,7 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { if (status != SYSPARAM_OK) return status; memcpy(&value_ctx, ctx, sizeof(value_ctx)); - status = _find_entry(&value_ctx, ctx->entry.id | 0x80); + status = _find_value(&value_ctx, ctx->entry.idflags); if (status < 0) return status; if (status == SYSPARAM_NOTFOUND) continue; @@ -821,7 +1023,13 @@ sysparam_status_t sysparam_iter_next(sysparam_iter_t *iter) { // Null-terminate the value (just in case) iter->value[value_ctx.entry.len] = 0; iter->value_len = value_ctx.entry.len; - debug(2, "iter_next: (0x%08x) '%s' = (0x%08x) '%s' (%d)", ctx->addr, iter->key, value_ctx.addr, iter->value, iter->value_len); + if (value_ctx.entry.idflags & ENTRY_FLAG_BINARY) { + iter->binary = true; + debug(2, "iter_next: (0x%08x) '%s' = (0x%08x) (%d)", ctx->addr, iter->key, value_ctx.addr, iter->value_len); + } else { + iter->binary = false; + debug(2, "iter_next: (0x%08x) '%s' = (0x%08x) '%s' (%d)", ctx->addr, iter->key, value_ctx.addr, iter->value, iter->value_len); + } return SYSPARAM_OK; } diff --git a/examples/sysparam_editor/Makefile b/examples/sysparam_editor/Makefile index a7843f3..a774b68 100644 --- a/examples/sysparam_editor/Makefile +++ b/examples/sysparam_editor/Makefile @@ -8,7 +8,7 @@ include ../../common.mk # `make dump-flash` can be used to view the current contents of the sysparam # regions in flash. dump-flash: - esptool.py read_flash 0x1fa000 4096 r1.bin + esptool.py read_flash 0x1f8000 8192 r1.bin hexdump -C r1.bin - esptool.py read_flash 0x1fb000 4096 r2.bin + esptool.py read_flash 0x1fa000 8192 r2.bin hexdump -C r2.bin diff --git a/examples/sysparam_editor/sysparam_editor.c b/examples/sysparam_editor/sysparam_editor.c index ac8fee1..d290eab 100644 --- a/examples/sysparam_editor/sysparam_editor.c +++ b/examples/sysparam_editor/sysparam_editor.c @@ -5,11 +5,19 @@ #include #include -#define CMD_BUF_SIZE 512 +#define CMD_BUF_SIZE 5000 -// This is just below the upper-4 sdk-reserved sectors for a 16mbit flash -// Note that the sysparam area will take up two sectors (0x1FAxxx and 0x1FBxxx) -#define SYSPARAM_ADDR 0x1fa000 +// Number of (4K) sectors that make up a sysparam area. Total sysparam data +// cannot be larger than half this amount. +// Note that if there is already a sysparam area created with a different size, +// that will continue to be used (if it can be found). This value is only used +// when creating/reformatting the sysparam area. +#define SYSPARAM_SECTORS 4 + +// This places the sysparam region just below the upper-4 sdk-reserved sectors +// for a 16mbit flash +#define FLASH_TOP 0x1fc000 +#define SYSPARAM_ADDR (FLASH_TOP - (SYSPARAM_SECTORS * 4096)) const int status_base = -6; const char *status_messages[] = { @@ -27,11 +35,12 @@ const char *status_messages[] = { void usage(void) { printf( "Available commands:\n" - " ? -- Query the value of \n" - " = -- Set to \n" - " dump -- Show all currently set keys/values\n" - " reformat -- Reinitialize (clear) the sysparam area\n" - " help -- Show this help screen\n" + " ? -- Query the value of \n" + " = -- Set to text \n" + " : -- Set to binary value represented as hex\n" + " dump -- Show all currently set keys/values\n" + " reformat -- Reinitialize (clear) the sysparam area\n" + " help -- Show this help screen\n" ); } @@ -63,6 +72,23 @@ size_t tty_readline(char *buffer, size_t buf_size, bool echo) { return i; } +void print_text_value(char *key, char *value) { + printf(" '%s' = '%s'\n", key, value); +} + +void print_binary_value(char *key, uint8_t *value, size_t len) { + size_t i; + + printf(" %s:", key); + for (i = 0; i < len; i++) { + if (!(i & 0x0f)) { + printf("\n "); + } + printf(" %02x", value[i]); + } + printf("\n"); +} + sysparam_status_t dump_params(void) { sysparam_status_t status; sysparam_iter_t iter; @@ -72,7 +98,11 @@ sysparam_status_t dump_params(void) { while (true) { status = sysparam_iter_next(&iter); if (status != SYSPARAM_OK) break; - printf(" %s=%s\n", iter.key, iter.value); + if (!iter.binary) { + print_text_value(iter.key, (char *)iter.value); + } else { + print_binary_value(iter.key, iter.value, iter.value_len); + } } sysparam_iter_end(&iter); @@ -85,28 +115,69 @@ sysparam_status_t dump_params(void) { } } +uint8_t *parse_hexdata(char *string, size_t *result_length) { + size_t string_len = strlen(string); + uint8_t *buf = malloc(string_len / 2); + uint8_t c; + int i, j; + bool digit = false; + + j = 0; + for (i = 0; string[i]; i++) { + c = string[i]; + if (c >= 0x30 && c <= 0x39) { + c &= 0x0f; + } else if (c >= 0x41 && c <= 0x46) { + c -= 0x37; + } else if (c >= 0x61 && c <= 0x66) { + c -= 0x57; + } else if (c == ' ') { + continue; + } else { + free(buf); + return NULL; + } + if (!digit) { + buf[j] = c << 4; + } else { + buf[j++] |= c; + } + digit = !digit; + } + if (digit) { + free(buf); + return NULL; + } + *result_length = j; + return buf; +} + void sysparam_editor_task(void *pvParameters) { char *cmd_buffer = malloc(CMD_BUF_SIZE); sysparam_status_t status; char *value; + uint8_t *bin_value; size_t len; + uint8_t *data; if (!cmd_buffer) { printf("ERROR: Cannot allocate command buffer!\n"); return; } + printf("\nWelcome to the system parameter editor! Enter 'help' for more information.\n\n"); + // NOTE: Eventually, this initialization part will be done automatically on // system startup, so the app won't need to do it. printf("Initializing sysparam...\n"); - status = sysparam_init(SYSPARAM_ADDR); + status = sysparam_init(SYSPARAM_ADDR, FLASH_TOP); printf("(status %d)\n", status); if (status == SYSPARAM_NOTFOUND) { printf("Trying to create new sysparam area...\n"); - status = sysparam_create_area(SYSPARAM_ADDR, false); + status = sysparam_create_area(SYSPARAM_ADDR, SYSPARAM_SECTORS, false); printf("(status %d)\n", status); if (status == SYSPARAM_OK) { - status = sysparam_init(SYSPARAM_ADDR); + status = sysparam_init(SYSPARAM_ADDR, 0); printf("(status %d)\n", status); } } @@ -121,23 +192,40 @@ void sysparam_editor_task(void *pvParameters) { printf("Querying '%s'...\n", cmd_buffer); status = sysparam_get_string(cmd_buffer, &value); if (status == SYSPARAM_OK) { - printf(" '%s' = '%s'\n", cmd_buffer, value); + print_text_value(cmd_buffer, value); + free(value); + } else if (status == SYSPARAM_PARSEFAILED) { + // This means it's actually a binary value + status = sysparam_get_data(cmd_buffer, &bin_value, &len, NULL); + if (status == SYSPARAM_OK) { + print_binary_value(cmd_buffer, bin_value, len); + free(value); + } } - free(value); } else if ((value = strchr(cmd_buffer, '='))) { *value++ = 0; printf("Setting '%s' to '%s'...\n", cmd_buffer, value); status = sysparam_set_string(cmd_buffer, value); + } else if ((value = strchr(cmd_buffer, ':'))) { + *value++ = 0; + data = parse_hexdata(value, &len); + if (value) { + printf("Setting '%s' to binary data...\n", cmd_buffer); + status = sysparam_set_data(cmd_buffer, data, len, true); + free(data); + } else { + printf("Error: Unable to parse hex data\n"); + } } else if (!strcmp(cmd_buffer, "dump")) { printf("Dumping all params:\n"); status = dump_params(); } else if (!strcmp(cmd_buffer, "reformat")) { printf("Re-initializing region...\n"); - status = sysparam_create_area(SYSPARAM_ADDR, true); + status = sysparam_create_area(SYSPARAM_ADDR, SYSPARAM_SECTORS, true); if (status == SYSPARAM_OK) { // We need to re-init after wiping out the region we've been // using. - status = sysparam_init(SYSPARAM_ADDR); + status = sysparam_init(SYSPARAM_ADDR, 0); } } else if (!strcmp(cmd_buffer, "help")) { usage(); From 66589f5d3f83dbab745cfedb79680aaac583ac6b Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Mon, 9 May 2016 20:47:30 -0700 Subject: [PATCH 10/11] Add sysparam_get_info function --- core/include/sysparam.h | 20 ++++++++++++++++++++ core/sysparam.c | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/core/include/sysparam.h b/core/include/sysparam.h index 9854f60..9260e73 100644 --- a/core/include/sysparam.h +++ b/core/include/sysparam.h @@ -3,6 +3,10 @@ #include +#ifndef DEFAULT_SYSPARAM_SECTORS +#define DEFAULT_SYSPARAM_SECTORS 4 +#endif + /** @file sysparam.h * * Read/write "system parameters" to persistent flash. @@ -112,6 +116,22 @@ sysparam_status_t sysparam_init(uint32_t base_addr, uint32_t top_addr); */ sysparam_status_t sysparam_create_area(uint32_t base_addr, uint16_t num_sectors, bool force); +/** Get the start address and size of the currently active sysparam area + * + * Fills in `base_addr` and `num_sectors` with the location and size of the + * currently active sysparam area. The returned values correspond to the + * arguments passed to the sysparam_create_area() call when the area was + * originally created. + * + * @param[out] base_addr The flash address at which the sysparam area starts + * @param[out] num_sectors The number of flash sectors used by the sysparam + * area + * + * @retval ::SYSPARAM_OK Completed successfully + * @retval ::SYSPARAM_ERR_NOINIT No current sysparam area is active + */ +sysparam_status_t sysparam_get_info(uint32_t *base_addr, uint32_t *num_sectors); + /** Get the value associated with a key * * This is the core "get value" function. It will retrieve the value for the diff --git a/core/sysparam.c b/core/sysparam.c index 0aa2cb2..cb08ac3 100644 --- a/core/sysparam.c +++ b/core/sysparam.c @@ -615,6 +615,14 @@ sysparam_status_t sysparam_create_area(uint32_t base_addr, uint16_t num_sectors, return SYSPARAM_OK; } +sysparam_status_t sysparam_get_info(uint32_t *base_addr, uint32_t *num_sectors) { + if (!_sysparam_info.cur_base) return SYSPARAM_ERR_NOINIT; + + *base_addr = min(_sysparam_info.cur_base, _sysparam_info.alt_base); + *num_sectors = (_sysparam_info.region_size / sdk_flashchip.sector_size) * 2; + return SYSPARAM_OK; +} + sysparam_status_t sysparam_get_data(const char *key, uint8_t **destptr, size_t *actual_length, bool *is_binary) { struct sysparam_context ctx; sysparam_status_t status; From 23f13db3aedeb7c6e8207b5467cd9399fbb16f57 Mon Sep 17 00:00:00 2001 From: Alex Stewart Date: Mon, 9 May 2016 20:48:06 -0700 Subject: [PATCH 11/11] Add sysparam initialization to app_main.c --- core/app_main.c | 17 +++++++++ examples/sysparam_editor/sysparam_editor.c | 43 ++++++++-------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/core/app_main.c b/core/app_main.c index 7a9bf8d..34a39b9 100644 --- a/core/app_main.c +++ b/core/app_main.c @@ -25,6 +25,7 @@ #include "espressif/esp_common.h" #include "sdk_internal.h" +#include "sysparam.h" /* This is not declared in any header file (but arguably should be) */ @@ -180,6 +181,8 @@ void IRAM sdk_user_start(void) { uint32_t cksum_len; uint32_t cksum_value; uint32_t ic_flash_addr; + uint32_t sysparam_addr; + sysparam_status_t status; SPI(0).USER0 |= SPI_USER0_CS_SETUP; sdk_SPIRead(0, buf32, 4); @@ -245,6 +248,20 @@ void IRAM sdk_user_start(void) { } memcpy(&sdk_g_ic.s, buf32, sizeof(struct sdk_g_ic_saved_st)); + // By default, put the sysparam region just below the config sectors at the + // top of the flash space + sysparam_addr = flash_size - (4 + DEFAULT_SYSPARAM_SECTORS) * sdk_flashchip.sector_size; + status = sysparam_init(sysparam_addr, flash_size); + if (status == SYSPARAM_NOTFOUND) { + status = sysparam_create_area(sysparam_addr, DEFAULT_SYSPARAM_SECTORS, false); + if (status == SYSPARAM_OK) { + status = sysparam_init(sysparam_addr, 0); + } + } + if (status != SYSPARAM_OK) { + printf("WARNING: Could not initialize sysparams (%d)!\n", status); + } + user_start_phase2(); } diff --git a/examples/sysparam_editor/sysparam_editor.c b/examples/sysparam_editor/sysparam_editor.c index d290eab..22f6190 100644 --- a/examples/sysparam_editor/sysparam_editor.c +++ b/examples/sysparam_editor/sysparam_editor.c @@ -5,20 +5,10 @@ #include #include +#include + #define CMD_BUF_SIZE 5000 -// Number of (4K) sectors that make up a sysparam area. Total sysparam data -// cannot be larger than half this amount. -// Note that if there is already a sysparam area created with a different size, -// that will continue to be used (if it can be found). This value is only used -// when creating/reformatting the sysparam area. -#define SYSPARAM_SECTORS 4 - -// This places the sysparam region just below the upper-4 sdk-reserved sectors -// for a 16mbit flash -#define FLASH_TOP 0x1fc000 -#define SYSPARAM_ADDR (FLASH_TOP - (SYSPARAM_SECTORS * 4096)) - const int status_base = -6; const char *status_messages[] = { "SYSPARAM_ERR_NOMEM", @@ -159,6 +149,7 @@ void sysparam_editor_task(void *pvParameters) { uint8_t *bin_value; size_t len; uint8_t *data; + uint32_t base_addr, num_sectors; if (!cmd_buffer) { printf("ERROR: Cannot allocate command buffer!\n"); @@ -167,21 +158,17 @@ void sysparam_editor_task(void *pvParameters) { printf("\nWelcome to the system parameter editor! Enter 'help' for more information.\n\n"); - // NOTE: Eventually, this initialization part will be done automatically on - // system startup, so the app won't need to do it. - printf("Initializing sysparam...\n"); - status = sysparam_init(SYSPARAM_ADDR, FLASH_TOP); - printf("(status %d)\n", status); - if (status == SYSPARAM_NOTFOUND) { - printf("Trying to create new sysparam area...\n"); - status = sysparam_create_area(SYSPARAM_ADDR, SYSPARAM_SECTORS, false); - printf("(status %d)\n", status); - if (status == SYSPARAM_OK) { - status = sysparam_init(SYSPARAM_ADDR, 0); - printf("(status %d)\n", status); - } + status = sysparam_get_info(&base_addr, &num_sectors); + if (status == SYSPARAM_OK) { + printf("[current sysparam region is at 0x%08x (%d sectors)]\n", base_addr, num_sectors); + } else { + printf("[NOTE: No current sysparam region (initialization problem during boot?)]\n"); + // Default to the same place/size as the normal system initialization + // stuff, so if the user uses this utility to reformat it, it will put + // it somewhere the system will find it later + num_sectors = DEFAULT_SYSPARAM_SECTORS; + base_addr = sdk_flashchip.chip_size - (4 + num_sectors) * sdk_flashchip.sector_size; } - while (true) { printf("==> "); len = tty_readline(cmd_buffer, CMD_BUF_SIZE, true); @@ -221,11 +208,11 @@ void sysparam_editor_task(void *pvParameters) { status = dump_params(); } else if (!strcmp(cmd_buffer, "reformat")) { printf("Re-initializing region...\n"); - status = sysparam_create_area(SYSPARAM_ADDR, SYSPARAM_SECTORS, true); + status = sysparam_create_area(base_addr, num_sectors, true); if (status == SYSPARAM_OK) { // We need to re-init after wiping out the region we've been // using. - status = sysparam_init(SYSPARAM_ADDR, 0); + status = sysparam_init(base_addr, 0); } } else if (!strcmp(cmd_buffer, "help")) { usage();