Simplify configuration dumping

Prior to this change, there was a lot of duplicated code within the dump
routines, which would call out to a common `print_section` routine that
had global state. This causes problems from a multi-threaded perspective
in that multiple calls to `x52d_config_save_file` were not MT-safe.

In addition, the dump logic was written such that it could only be used
in the config dump. It is desired that we add functionality to return
the formatted config value as a string in a different part of the code
as well.

This change brings in the shared state into a stack variable, and
changes the dump functions to return a const char *, thereby allowing
for greater reuse, as well as getting rid of the shared state. However,
there is still a little bit of shared state in the `int_dumper` routine.
This can be ignored for now, but we should possibly figure out how to
get rid of the shared state altogether.
reverse-scroll
nirenjan 2021-11-02 23:15:03 -07:00
parent 6d78ab1940
commit 6c3efa44f5
1 changed files with 36 additions and 56 deletions

View File

@ -19,106 +19,74 @@
#include "x52d_config.h" #include "x52d_config.h"
#include "x52d_const.h" #include "x52d_const.h"
static char *current_section = NULL;
// Print the current section to the file
static void print_section(FILE *cfg, const char *section)
{
if (current_section == NULL || strcasecmp(current_section, section)) {
if (current_section != NULL) {
free(current_section);
}
current_section = strdup(section);
PINELOG_TRACE("Printing section header %s", section);
fprintf(cfg, "[%s]\n", section);
}
}
// Create a pointer "name" of type "type", which stores the value of the // Create a pointer "name" of type "type", which stores the value of the
// corresponding element within the config struct. // corresponding element within the config struct.
#define CONFIG_PTR(type, name) type name = (type)((uintptr_t)cfg + offset) #define CONFIG_PTR(type, name) type name = (type)((uintptr_t)cfg + offset)
// Check if the parameters are all valid // Check if the parameters are all valid
#define CHECK_PARAMS() do { if (cfg == NULL || section == NULL || key == NULL) { return EINVAL; } } while(0) #define CHECK_PARAMS() do { if (cfg == NULL || section == NULL || key == NULL) { return NULL; } } while(0)
static int bool_dumper(FILE *file, const char *section, const char *key, const struct x52d_config *cfg, size_t offset) static const char * bool_dumper(const char *section, const char *key, const struct x52d_config *cfg, size_t offset)
{ {
CONFIG_PTR(bool *, config); CONFIG_PTR(bool *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing bool value %s.%s from offset %lu value = %d", PINELOG_TRACE("Printing bool value %s.%s from offset %lu value = %d",
section, key, offset, *config); section, key, offset, *config);
fprintf(file, "%s = %s\n", key, *config ? "true" : "false"); return *config ? "true" : "false";
return 0;
} }
static int string_dumper(FILE *file, const char *section, const char *key, struct x52d_config *cfg, size_t offset) static const char * string_dumper(const char *section, const char *key, struct x52d_config *cfg, size_t offset)
{ {
CONFIG_PTR(char *, config); CONFIG_PTR(char *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing string value %s.%s from offset %lu value = %s", PINELOG_TRACE("Printing string value %s.%s from offset %lu value = %s",
section, key, offset, config); section, key, offset, config);
fprintf(file, "%s = %s\n", key, config); return config;
return 0;
} }
static int int_dumper(FILE *file, const char *section, const char *key, struct x52d_config *cfg, size_t offset) static const char * int_dumper(const char *section, const char *key, struct x52d_config *cfg, size_t offset)
{ {
static char dump[256];
CONFIG_PTR(int *, config); CONFIG_PTR(int *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing int value %s.%s from offset %lu value = %d", PINELOG_TRACE("Printing int value %s.%s from offset %lu value = %d",
section, key, offset, *config); section, key, offset, *config);
fprintf(file, "%s = %d\n", key, *config); snprintf(dump, sizeof(dump), "%d", *config);
return 0; return dump;
} }
static int led_dumper(FILE *file, const char *section, const char *key, struct x52d_config *cfg, size_t offset) static const char * led_dumper(const char *section, const char *key, struct x52d_config *cfg, size_t offset)
{ {
CONFIG_PTR(libx52_led_state *, config); CONFIG_PTR(libx52_led_state *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing led value %s.%s from offset %lu value = %d", PINELOG_TRACE("Printing led value %s.%s from offset %lu value = %d",
section, key, offset, *config); section, key, offset, *config);
fprintf(file, "%s = %s\n", key, libx52_led_state_to_str(*config)); return libx52_led_state_to_str(*config);
return 0;
} }
static int clock_format_dumper(FILE *file, const char *section, const char *key, struct x52d_config *cfg, size_t offset) static const char * clock_format_dumper(const char *section, const char *key, struct x52d_config *cfg, size_t offset)
{ {
CONFIG_PTR(libx52_clock_format *, config); CONFIG_PTR(libx52_clock_format *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing clock format value %s.%s from offset %lu value = %d", PINELOG_TRACE("Printing clock format value %s.%s from offset %lu value = %d",
section, key, offset, *config); section, key, offset, *config);
fprintf(file, "%s = %s\n", key, libx52_clock_format_to_str(*config)); return libx52_clock_format_to_str(*config);
return 0;
} }
static int date_format_dumper(FILE *file, const char *section, const char *key, struct x52d_config *cfg, size_t offset) static const char * date_format_dumper(const char *section, const char *key, struct x52d_config *cfg, size_t offset)
{ {
CONFIG_PTR(libx52_date_format *, config); CONFIG_PTR(libx52_date_format *, config);
CHECK_PARAMS(); CHECK_PARAMS();
print_section(file, section);
PINELOG_TRACE("Printing date format value %s.%s from offset %lu value = %d", PINELOG_TRACE("Printing date format value %s.%s from offset %lu value = %d",
section, key, offset, *config); section, key, offset, *config);
fprintf(file, "%s = %s\n", key, libx52_date_format_to_str(*config)); return libx52_date_format_to_str(*config);
return 0;
} }
#undef CHECK_PARAMS #undef CHECK_PARAMS
@ -126,8 +94,10 @@ static int date_format_dumper(FILE *file, const char *section, const char *key,
int x52d_config_save_file(struct x52d_config *cfg, const char *cfg_file) int x52d_config_save_file(struct x52d_config *cfg, const char *cfg_file)
{ {
int rc;
FILE *cfg_fp; FILE *cfg_fp;
char *current_section = NULL;
const char *value;
if (cfg == NULL || cfg_file == NULL) { if (cfg == NULL || cfg_file == NULL) {
return EINVAL; return EINVAL;
} }
@ -139,26 +109,36 @@ int x52d_config_save_file(struct x52d_config *cfg, const char *cfg_file)
cfg_fp = fopen(cfg_file, "w"); cfg_fp = fopen(cfg_file, "w");
if (cfg_fp == NULL) { if (cfg_fp == NULL) {
rc = errno;
PINELOG_ERROR(_("Unable to save config file %s - code %d: %s"), PINELOG_ERROR(_("Unable to save config file %s - code %d: %s"),
cfg_file, rc, strerror(rc)); cfg_file, errno, strerror(errno));
return rc; return 1;
} }
PINELOG_TRACE("Saving configuration to file %s", cfg_file); PINELOG_TRACE("Saving configuration to file %s", cfg_file);
#define CFG(section, key, name, type, def) do { \ #define CFG(section, key, name, type, def) do { \
if (current_section == NULL || strcasecmp(current_section, #section)) { \
if (current_section != NULL) { \
free(current_section); \
} \
current_section = strdup(#section); \
PINELOG_TRACE("Printing section header %s", #section); \
fprintf(cfg_fp, "[%s]\n", #section); \
} \
PINELOG_TRACE("Dumping " #section "." #key " to file %s", cfg_file); \ PINELOG_TRACE("Dumping " #section "." #key " to file %s", cfg_file); \
rc = type ## _dumper(cfg_fp, #section, #key, cfg, offsetof(struct x52d_config, name)); \ value = type ## _dumper(#section, #key, cfg, offsetof(struct x52d_config, name)); \
if (rc) { \ if (value == NULL) { \
PINELOG_ERROR(_("Failed to dump %s.%s to config file %s - code %d: %s"), \ PINELOG_ERROR(_("Failed to dump %s.%s to config file %s"), \
#section, #key, cfg_file, rc, strerror(rc)); \ #section, #key, cfg_file); \
goto exit_dump; \ goto exit_dump; \
} else { \
fprintf(cfg_fp, "%s = %s\n", #key, value); \
} \ } \
} while (0); } while (0);
#include "x52d_config.def" #include "x52d_config.def"
exit_dump: exit_dump:
free(current_section);
fclose(cfg_fp); fclose(cfg_fp);
return rc; return (value == NULL);
} }