| Message ID | 1519497068-12484-1-git-send-email-sbabic@denx.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mongoose: fix parsing of configuration file | expand |
Hi Stefano, Am 24.02.2018 um 19:31 schrieb Stefano Babic: > GET_FIELD_STRING_RESET() ensure that the returned string is empty > after parsing an entry, else the string contains last parsed element. Thanks for finding and fixing the mistake. The same error exists in the downloader.c file. The function is suboptimal because it doesn't return an error nor an valid string in the error case. > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- > mongoose/mongoose_interface.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index 89a51f3..60914d3 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -477,27 +477,27 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat > struct mongoose_options *opts = (struct mongoose_options *)data; > char tmp[128]; > > - GET_FIELD_STRING(LIBCFG_PARSER, elem, "document_root", tmp); > + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "document_root", tmp); > if (strlen(tmp)) { > opts->root = strdup(tmp); > } > > - GET_FIELD_STRING(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); > + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); > if (strlen(tmp)) { > opts->listing = strdup(tmp); > } > > - GET_FIELD_STRING(LIBCFG_PARSER, elem, "listening_ports", tmp); > + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); > if (strlen(tmp)) { > opts->port = strdup(tmp); > } > #if MG_ENABLE_SSL > - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate", tmp); > + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "ssl_certificate", tmp); > if (strlen(tmp)) { > opts->ssl_cert = strdup(tmp); > } > > - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate_key", tmp); > + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "ssl_certificate_key", tmp); > if (strlen(tmp)) { > opts->ssl_key = strdup(tmp); > } Acked-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> Best regards Stefan
Hi Stefan, On 25/02/2018 16:02, Stefan Herbrechtsmeier wrote: > Hi Stefano, > > Am 24.02.2018 um 19:31 schrieb Stefano Babic: >> GET_FIELD_STRING_RESET() ensure that the returned string is empty >> after parsing an entry, else the string contains last parsed element. > Thanks for finding and fixing the mistake. > > The same error exists in the downloader.c file. > Thanks - I send a similar patch. > The function is suboptimal because it doesn't return an error nor an > valid string in the error case. True, but a not found item does not mean an error. It is ok that the configuration file does not contain all entries. Regards, Stefano > >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> --- >> mongoose/mongoose_interface.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/mongoose/mongoose_interface.c >> b/mongoose/mongoose_interface.c >> index 89a51f3..60914d3 100644 >> --- a/mongoose/mongoose_interface.c >> +++ b/mongoose/mongoose_interface.c >> @@ -477,27 +477,27 @@ static int mongoose_settings(void *elem, void >> __attribute__ ((__unused__)) *dat >> struct mongoose_options *opts = (struct mongoose_options *)data; >> char tmp[128]; >> - GET_FIELD_STRING(LIBCFG_PARSER, elem, "document_root", tmp); >> + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "document_root", tmp); >> if (strlen(tmp)) { >> opts->root = strdup(tmp); >> } >> - GET_FIELD_STRING(LIBCFG_PARSER, elem, >> "enable_directory_listing", tmp); >> + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, >> "enable_directory_listing", tmp); >> if (strlen(tmp)) { >> opts->listing = strdup(tmp); >> } >> - GET_FIELD_STRING(LIBCFG_PARSER, elem, "listening_ports", tmp); >> + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); >> if (strlen(tmp)) { >> opts->port = strdup(tmp); >> } >> #if MG_ENABLE_SSL >> - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate", tmp); >> + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "ssl_certificate", tmp); >> if (strlen(tmp)) { >> opts->ssl_cert = strdup(tmp); >> } >> - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate_key", tmp); >> + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, >> "ssl_certificate_key", tmp); >> if (strlen(tmp)) { >> opts->ssl_key = strdup(tmp); >> } > > Acked-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > Best regards > Stefan >
Hi Stefano, Am 25.02.2018 um 19:01 schrieb Stefano Babic: > On 25/02/2018 16:02, Stefan Herbrechtsmeier wrote: >> Am 24.02.2018 um 19:31 schrieb Stefano Babic: >>> GET_FIELD_STRING_RESET() ensure that the returned string is empty >>> after parsing an entry, else the string contains last parsed element. [snip] >> The function is suboptimal because it doesn't return an error nor an >> valid string in the error case. > True, but a not found item does not mean an error. It is ok that the > configuration file does not contain all entries. Yes but it isn't good to just do nothing if the entry is missing. At the moment the caller of the get_field_string_with_size function needs to set the passed string to a well-known value before the function call to handle this case. I think that more users of the GET_FIELD_STRING function needs to be converted to the GET_FIELD_STRING_RESET function because they don't explicitly initialize the value before the function. Best regards Stefan
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c index 89a51f3..60914d3 100644 --- a/mongoose/mongoose_interface.c +++ b/mongoose/mongoose_interface.c @@ -477,27 +477,27 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat struct mongoose_options *opts = (struct mongoose_options *)data; char tmp[128]; - GET_FIELD_STRING(LIBCFG_PARSER, elem, "document_root", tmp); + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "document_root", tmp); if (strlen(tmp)) { opts->root = strdup(tmp); } - GET_FIELD_STRING(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); if (strlen(tmp)) { opts->listing = strdup(tmp); } - GET_FIELD_STRING(LIBCFG_PARSER, elem, "listening_ports", tmp); + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); if (strlen(tmp)) { opts->port = strdup(tmp); } #if MG_ENABLE_SSL - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate", tmp); + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "ssl_certificate", tmp); if (strlen(tmp)) { opts->ssl_cert = strdup(tmp); } - GET_FIELD_STRING(LIBCFG_PARSER, elem, "ssl_certificate_key", tmp); + GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "ssl_certificate_key", tmp); if (strlen(tmp)) { opts->ssl_key = strdup(tmp); }
GET_FIELD_STRING_RESET() ensure that the returned string is empty after parsing an entry, else the string contains last parsed element. Signed-off-by: Stefano Babic <sbabic@denx.de> --- mongoose/mongoose_interface.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)