| Message ID | 20200918125528.31243-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | core: Refactor config file reading to fail early | expand |
On 18.09.20 14:55, Christian Storm wrote: > Refactor to fail early if config file cannot be read or the > mandatory 'globals' section is absent. > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/swupdate.c | 38 ++++++++++++++++++------------------- > corelib/swupdate_settings.c | 2 -- > 2 files changed, 18 insertions(+), 22 deletions(-) > > diff --git a/core/swupdate.c b/core/swupdate.c > index 07c79f9..49c33d0 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -719,35 +719,33 @@ int main(int argc, char **argv) > > /* Load configuration file */ > if (cfgfname != NULL) { > - if (read_module_settings(cfgfname, "globals", > - read_globals_settings, &swcfg)) { > + /* > + * 'globals' section is mandatory if configuration file is specified. > + */ > + int ret = read_module_settings(cfgfname, "globals", > + read_globals_settings, &swcfg); > + if (ret != 0) { > + /* > + * Exit on -ENODATA or -EINVAL errors. > + */ > fprintf(stderr, > - "Error parsing configuration file, exiting.\n"); > + "Error parsing configuration file: %s, exiting.\n", > + ret == -ENODATA ? "'globals' section missing" > + : "cannot read"); > exit(EXIT_FAILURE); > } > > - loglevel = swcfg.globals.loglevel; > - if (swcfg.globals.verbose) > - loglevel = TRACELEVEL; > + loglevel = swcfg.globals.verbose ? TRACELEVEL : swcfg.globals.loglevel; > > /* > - * logcolors is optional, ignore error code > - * if not found > + * The following sections are optional, hence -ENODATA error code is > + * ignored if the section is not found. -EINVAL will not happen here. > */ > (void)read_module_settings(cfgfname, "logcolors", > - read_console_settings, &swcfg); > + read_console_settings, &swcfg); > > - int ret = read_module_settings(cfgfname, "processes", > - read_processes_settings, > - &swcfg); > - /* > - * ignore other errors, check only if file is parsed > - */ > - if (ret == -EINVAL) { > - fprintf(stderr, > - "Error parsing configuration file, exiting.\n"); > - exit(EXIT_FAILURE); > - } > + (void)read_module_settings(cfgfname, "processes", > + read_processes_settings, &swcfg); > } > > printf("%s\n", BANNER); > diff --git a/corelib/swupdate_settings.c b/corelib/swupdate_settings.c > index 2885d8e..b0d3344 100644 > --- a/corelib/swupdate_settings.c > +++ b/corelib/swupdate_settings.c > @@ -53,7 +53,6 @@ static int read_settings_file(config_t *cfg, const char *filename) > { > int ret; > > - /* Read the file. If there is an error, report it and exit. */ > ret = config_read_file(cfg, filename); > if (ret != CONFIG_TRUE) { > fprintf(stderr, "%s ", config_error_file(cfg)); > @@ -78,7 +77,6 @@ int read_module_settings(const char *filename, const char *module, settings_call > memset(&cfg, 0, sizeof(cfg)); > config_init(&cfg); > > - /* Read the file. If there is an error, report it and exit. */ > if (read_settings_file(&cfg, filename) != CONFIG_TRUE) { > config_destroy(&cfg); > ERROR("Error reading configuration file, skipping...."); > Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
On 18.09.20 15:05, Stefano Babic wrote: > On 18.09.20 14:55, Christian Storm wrote: >> Refactor to fail early if config file cannot be read or the >> mandatory 'globals' section is absent. >> >> Signed-off-by: Christian Storm <christian.storm@siemens.com> >> --- >> core/swupdate.c | 38 ++++++++++++++++++------------------- >> corelib/swupdate_settings.c | 2 -- >> 2 files changed, 18 insertions(+), 22 deletions(-) >> >> diff --git a/core/swupdate.c b/core/swupdate.c >> index 07c79f9..49c33d0 100644 >> --- a/core/swupdate.c >> +++ b/core/swupdate.c >> @@ -719,35 +719,33 @@ int main(int argc, char **argv) >> >> /* Load configuration file */ >> if (cfgfname != NULL) { >> - if (read_module_settings(cfgfname, "globals", >> - read_globals_settings, &swcfg)) { >> + /* >> + * 'globals' section is mandatory if configuration file is specified. >> + */ >> + int ret = read_module_settings(cfgfname, "globals", >> + read_globals_settings, &swcfg); >> + if (ret != 0) { >> + /* >> + * Exit on -ENODATA or -EINVAL errors. >> + */ >> fprintf(stderr, >> - "Error parsing configuration file, exiting.\n"); >> + "Error parsing configuration file: %s, exiting.\n", >> + ret == -ENODATA ? "'globals' section missing" >> + : "cannot read"); >> exit(EXIT_FAILURE); >> } >> >> - loglevel = swcfg.globals.loglevel; >> - if (swcfg.globals.verbose) >> - loglevel = TRACELEVEL; >> + loglevel = swcfg.globals.verbose ? TRACELEVEL : swcfg.globals.loglevel; >> >> /* >> - * logcolors is optional, ignore error code >> - * if not found >> + * The following sections are optional, hence -ENODATA error code is >> + * ignored if the section is not found. -EINVAL will not happen here. >> */ >> (void)read_module_settings(cfgfname, "logcolors", >> - read_console_settings, &swcfg); >> + read_console_settings, &swcfg); >> >> - int ret = read_module_settings(cfgfname, "processes", >> - read_processes_settings, >> - &swcfg); >> - /* >> - * ignore other errors, check only if file is parsed >> - */ >> - if (ret == -EINVAL) { >> - fprintf(stderr, >> - "Error parsing configuration file, exiting.\n"); >> - exit(EXIT_FAILURE); >> - } >> + (void)read_module_settings(cfgfname, "processes", >> + read_processes_settings, &swcfg); >> } >> >> printf("%s\n", BANNER); >> diff --git a/corelib/swupdate_settings.c b/corelib/swupdate_settings.c >> index 2885d8e..b0d3344 100644 >> --- a/corelib/swupdate_settings.c >> +++ b/corelib/swupdate_settings.c >> @@ -53,7 +53,6 @@ static int read_settings_file(config_t *cfg, const char *filename) >> { >> int ret; >> >> - /* Read the file. If there is an error, report it and exit. */ >> ret = config_read_file(cfg, filename); >> if (ret != CONFIG_TRUE) { >> fprintf(stderr, "%s ", config_error_file(cfg)); >> @@ -78,7 +77,6 @@ int read_module_settings(const char *filename, const char *module, settings_call >> memset(&cfg, 0, sizeof(cfg)); >> config_init(&cfg); >> >> - /* Read the file. If there is an error, report it and exit. */ >> if (read_settings_file(&cfg, filename) != CONFIG_TRUE) { >> config_destroy(&cfg); >> ERROR("Error reading configuration file, skipping...."); >> > > Acked-by: Stefano Babic <sbabic@denx.de> > Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/core/swupdate.c b/core/swupdate.c index 07c79f9..49c33d0 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -719,35 +719,33 @@ int main(int argc, char **argv) /* Load configuration file */ if (cfgfname != NULL) { - if (read_module_settings(cfgfname, "globals", - read_globals_settings, &swcfg)) { + /* + * 'globals' section is mandatory if configuration file is specified. + */ + int ret = read_module_settings(cfgfname, "globals", + read_globals_settings, &swcfg); + if (ret != 0) { + /* + * Exit on -ENODATA or -EINVAL errors. + */ fprintf(stderr, - "Error parsing configuration file, exiting.\n"); + "Error parsing configuration file: %s, exiting.\n", + ret == -ENODATA ? "'globals' section missing" + : "cannot read"); exit(EXIT_FAILURE); } - loglevel = swcfg.globals.loglevel; - if (swcfg.globals.verbose) - loglevel = TRACELEVEL; + loglevel = swcfg.globals.verbose ? TRACELEVEL : swcfg.globals.loglevel; /* - * logcolors is optional, ignore error code - * if not found + * The following sections are optional, hence -ENODATA error code is + * ignored if the section is not found. -EINVAL will not happen here. */ (void)read_module_settings(cfgfname, "logcolors", - read_console_settings, &swcfg); + read_console_settings, &swcfg); - int ret = read_module_settings(cfgfname, "processes", - read_processes_settings, - &swcfg); - /* - * ignore other errors, check only if file is parsed - */ - if (ret == -EINVAL) { - fprintf(stderr, - "Error parsing configuration file, exiting.\n"); - exit(EXIT_FAILURE); - } + (void)read_module_settings(cfgfname, "processes", + read_processes_settings, &swcfg); } printf("%s\n", BANNER); diff --git a/corelib/swupdate_settings.c b/corelib/swupdate_settings.c index 2885d8e..b0d3344 100644 --- a/corelib/swupdate_settings.c +++ b/corelib/swupdate_settings.c @@ -53,7 +53,6 @@ static int read_settings_file(config_t *cfg, const char *filename) { int ret; - /* Read the file. If there is an error, report it and exit. */ ret = config_read_file(cfg, filename); if (ret != CONFIG_TRUE) { fprintf(stderr, "%s ", config_error_file(cfg)); @@ -78,7 +77,6 @@ int read_module_settings(const char *filename, const char *module, settings_call memset(&cfg, 0, sizeof(cfg)); config_init(&cfg); - /* Read the file. If there is an error, report it and exit. */ if (read_settings_file(&cfg, filename) != CONFIG_TRUE) { config_destroy(&cfg); ERROR("Error reading configuration file, skipping....");
Refactor to fail early if config file cannot be read or the mandatory 'globals' section is absent. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- core/swupdate.c | 38 ++++++++++++++++++------------------- corelib/swupdate_settings.c | 2 -- 2 files changed, 18 insertions(+), 22 deletions(-)