| Message ID | 1520629556-1353-1-git-send-email-sbabic@denx.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] mongoose: SEGV if -l is passed | expand |
Hi Stefano, Am 09.03.2018 um 22:05 schrieb Stefano Babic: > From: Lars Lockenvitz <Lars.Lockenvitz@smartray.de> > > cmdline option --listing does not get an argument, > but mongoose needs "yes" to activate this option > > Signed-off-by: Lars Lockenvitz <Lars.Lockenvitz@smartray.de> > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- > > Changes since V1: > - fix commit message s7ibut/but/ > - replace string with boolean for "listing" > > mongoose/mongoose_interface.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index 42a20f8..99af755 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -17,6 +17,7 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> > +#include <stdbool.h> > > #include <getopt.h> > > @@ -28,7 +29,6 @@ > > #include "mongoose.h" > > -#define MG_LISTING "no" > #define MG_PORT "8080" > #define MG_ROOT "." > > @@ -39,7 +39,7 @@ enum MONGOOSE_API_VERSION { > > struct mongoose_options { > char *root; > - char *listing; > + bool listing; > char *port; > char *global_auth_file; > char *auth_domain; > @@ -455,7 +455,7 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat > > GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); > if (strlen(tmp)) { > - opts->listing = strdup(tmp); > + opts->listing = !strcmp(tmp, "yes") ? true : false; > } Could this be changed to a get_field function with a bool value? > > GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); > @@ -511,7 +511,7 @@ void mongoose_print_help(void) > fprintf( > stdout, > "\tmongoose arguments:\n" > - "\t -l, --listing <port> : enable directory listing (default: %s)\n" > + "\t -l, --listing : enable directory listing\n" > "\t -p, --port <port> : server port number (default: %s)\n" > #if MG_ENABLE_SSL > "\t -s, --ssl : enable ssl support\n" > @@ -522,7 +522,7 @@ void mongoose_print_help(void) > "\t -a, --api-version [1|2] : set Web protocol API to v1 (legacy) or v2 (default v2)\n" > "\t --auth-domain : set authentication domain if any (default: none)\n" > "\t --global-auth-file : set authentication file if any (default: none)\n", > - MG_LISTING, MG_PORT, MG_ROOT); > + MG_PORT, MG_ROOT); > } > > int start_mongoose(const char *cfgfname, int argc, char *argv[]) > @@ -541,6 +541,10 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > memset(&opts, 0, sizeof(opts)); > /* Set default API version */ > opts.api_version = MONGOOSE_API_V2; > + > + /* No listing directory as default */ > + opts.listing = false; > + > if (cfgfname) { > read_module_settings(cfgfname, "webserver", mongoose_settings, &opts); > } > @@ -558,8 +562,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > opts.global_auth_file = strdup(optarg); > break; > case 'l': > - free(opts.listing); > - opts.listing = strdup(optarg); > + opts.listing = true; > break; > case 'p': > free(opts.port); > @@ -596,7 +599,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) > s_http_server_opts.document_root = > opts.root ? opts.root : MG_ROOT; > s_http_server_opts.enable_directory_listing = > - opts.listing ? opts.listing : MG_LISTING; > + opts.listing ? "yes" : "no"; > s_http_port = opts.port ? opts.port : MG_PORT; > s_http_server_opts.global_auth_file = opts.global_auth_file; > s_http_server_opts.auth_domain = opts.auth_domain; Best regards Stefan
Hi Stefan, On 11/03/2018 11:06, Stefan Herbrechtsmeier wrote: > Hi Stefano, > > Am 09.03.2018 um 22:05 schrieb Stefano Babic: >> From: Lars Lockenvitz <Lars.Lockenvitz@smartray.de> >> >> cmdline option --listing does not get an argument, >> but mongoose needs "yes" to activate this option >> >> Signed-off-by: Lars Lockenvitz <Lars.Lockenvitz@smartray.de> >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> --- >> >> Changes since V1: >> - fix commit message s7ibut/but/ >> - replace string with boolean for "listing" >> >> mongoose/mongoose_interface.c | 19 +++++++++++-------- >> 1 file changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c >> index 42a20f8..99af755 100644 >> --- a/mongoose/mongoose_interface.c >> +++ b/mongoose/mongoose_interface.c >> @@ -17,6 +17,7 @@ >> #include <stdlib.h> >> #include <string.h> >> #include <unistd.h> >> +#include <stdbool.h> >> >> #include <getopt.h> >> >> @@ -28,7 +29,6 @@ >> >> #include "mongoose.h" >> >> -#define MG_LISTING "no" >> #define MG_PORT "8080" >> #define MG_ROOT "." >> >> @@ -39,7 +39,7 @@ enum MONGOOSE_API_VERSION { >> >> struct mongoose_options { >> char *root; >> - char *listing; >> + bool listing; >> char *port; >> char *global_auth_file; >> char *auth_domain; >> @@ -455,7 +455,7 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat >> >> GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); >> if (strlen(tmp)) { >> - opts->listing = strdup(tmp); >> + opts->listing = !strcmp(tmp, "yes") ? true : false; >> } > > Could this be changed to a get_field function with a bool value? It could be, but this reflects the type in mongoose and it is more straightforward. mongoose requires "yes" or "no". It seems easier for users if we stick with a string into swupdate.cfg, even if it is could be converted internally. Regards, Stefano > >> >> GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); >> @@ -511,7 +511,7 @@ void mongoose_print_help(void) >> fprintf( >> stdout, >> "\tmongoose arguments:\n" >> - "\t -l, --listing <port> : enable directory listing (default: %s)\n" >> + "\t -l, --listing : enable directory listing\n" >> "\t -p, --port <port> : server port number (default: %s)\n" >> #if MG_ENABLE_SSL >> "\t -s, --ssl : enable ssl support\n" >> @@ -522,7 +522,7 @@ void mongoose_print_help(void) >> "\t -a, --api-version [1|2] : set Web protocol API to v1 (legacy) or v2 (default v2)\n" >> "\t --auth-domain : set authentication domain if any (default: none)\n" >> "\t --global-auth-file : set authentication file if any (default: none)\n", >> - MG_LISTING, MG_PORT, MG_ROOT); >> + MG_PORT, MG_ROOT); >> } >> >> int start_mongoose(const char *cfgfname, int argc, char *argv[]) >> @@ -541,6 +541,10 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) >> memset(&opts, 0, sizeof(opts)); >> /* Set default API version */ >> opts.api_version = MONGOOSE_API_V2; >> + >> + /* No listing directory as default */ >> + opts.listing = false; >> + >> if (cfgfname) { >> read_module_settings(cfgfname, "webserver", mongoose_settings, &opts); >> } >> @@ -558,8 +562,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) >> opts.global_auth_file = strdup(optarg); >> break; >> case 'l': >> - free(opts.listing); >> - opts.listing = strdup(optarg); >> + opts.listing = true; >> break; >> case 'p': >> free(opts.port); >> @@ -596,7 +599,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) >> s_http_server_opts.document_root = >> opts.root ? opts.root : MG_ROOT; >> s_http_server_opts.enable_directory_listing = >> - opts.listing ? opts.listing : MG_LISTING; >> + opts.listing ? "yes" : "no"; >> s_http_port = opts.port ? opts.port : MG_PORT; >> s_http_server_opts.global_auth_file = opts.global_auth_file; >> s_http_server_opts.auth_domain = opts.auth_domain; > > Best regards > Stefan > > -- > You received this message because you are subscribed to the Google > Groups "swupdate" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate+unsubscribe@googlegroups.com>. > To post to this group, send email to swupdate@googlegroups.com > <mailto:swupdate@googlegroups.com>. > For more options, visit https://groups.google.com/d/optout.
Hi Stefano, Am 11.03.2018 um 11:12 schrieb Stefano Babic: > >>> @@ -455,7 +455,7 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat >>> >>> GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); >>> if (strlen(tmp)) { >>> - opts->listing = strdup(tmp); >>> + opts->listing = !strcmp(tmp, "yes") ? true : false; >>> } >> Could this be changed to a get_field function with a bool value? > It could be, but this reflects the type in mongoose and it is more > straightforward. mongoose requires "yes" or "no". But this is an internal implementation detail with isn't known to our user. > It seems easier for > users if we stick with a string into swupdate.cfg, even if it is could > be converted internally. I find this inconsistent for the user because this is a simple switch but instead of a true he has to use a 'yes' and a 'Yes' or 'YES' doesn't work. Best regards, Stefan
On 11/03/2018 11:22, Stefan Herbrechtsmeier wrote: > Hi Stefano, > > Am 11.03.2018 um 11:12 schrieb Stefano Babic: >> >>>> @@ -455,7 +455,7 @@ static int mongoose_settings(void *elem, void >>>> __attribute__ ((__unused__)) *dat >>>> GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, >>>> "enable_directory_listing", tmp); >>>> if (strlen(tmp)) { >>>> - opts->listing = strdup(tmp); >>>> + opts->listing = !strcmp(tmp, "yes") ? true : false; >>>> } >>> Could this be changed to a get_field function with a bool value? >> It could be, but this reflects the type in mongoose and it is more >> straightforward. mongoose requires "yes" or "no". > > But this is an internal implementation detail with isn't known to our user. > >> It seems easier for >> users if we stick with a string into swupdate.cfg, even if it is could >> be converted internally. > > I find this inconsistent for the user because this is a simple switch > but instead of a true he has to use a 'yes' and a 'Yes' or 'YES' doesn't > work. mmhhh...right, then we have camelcase... I will fix this, thanks. Best regards, Stefano
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c index 42a20f8..99af755 100644 --- a/mongoose/mongoose_interface.c +++ b/mongoose/mongoose_interface.c @@ -17,6 +17,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <stdbool.h> #include <getopt.h> @@ -28,7 +29,6 @@ #include "mongoose.h" -#define MG_LISTING "no" #define MG_PORT "8080" #define MG_ROOT "." @@ -39,7 +39,7 @@ enum MONGOOSE_API_VERSION { struct mongoose_options { char *root; - char *listing; + bool listing; char *port; char *global_auth_file; char *auth_domain; @@ -455,7 +455,7 @@ static int mongoose_settings(void *elem, void __attribute__ ((__unused__)) *dat GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "enable_directory_listing", tmp); if (strlen(tmp)) { - opts->listing = strdup(tmp); + opts->listing = !strcmp(tmp, "yes") ? true : false; } GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "listening_ports", tmp); @@ -511,7 +511,7 @@ void mongoose_print_help(void) fprintf( stdout, "\tmongoose arguments:\n" - "\t -l, --listing <port> : enable directory listing (default: %s)\n" + "\t -l, --listing : enable directory listing\n" "\t -p, --port <port> : server port number (default: %s)\n" #if MG_ENABLE_SSL "\t -s, --ssl : enable ssl support\n" @@ -522,7 +522,7 @@ void mongoose_print_help(void) "\t -a, --api-version [1|2] : set Web protocol API to v1 (legacy) or v2 (default v2)\n" "\t --auth-domain : set authentication domain if any (default: none)\n" "\t --global-auth-file : set authentication file if any (default: none)\n", - MG_LISTING, MG_PORT, MG_ROOT); + MG_PORT, MG_ROOT); } int start_mongoose(const char *cfgfname, int argc, char *argv[]) @@ -541,6 +541,10 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) memset(&opts, 0, sizeof(opts)); /* Set default API version */ opts.api_version = MONGOOSE_API_V2; + + /* No listing directory as default */ + opts.listing = false; + if (cfgfname) { read_module_settings(cfgfname, "webserver", mongoose_settings, &opts); } @@ -558,8 +562,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) opts.global_auth_file = strdup(optarg); break; case 'l': - free(opts.listing); - opts.listing = strdup(optarg); + opts.listing = true; break; case 'p': free(opts.port); @@ -596,7 +599,7 @@ int start_mongoose(const char *cfgfname, int argc, char *argv[]) s_http_server_opts.document_root = opts.root ? opts.root : MG_ROOT; s_http_server_opts.enable_directory_listing = - opts.listing ? opts.listing : MG_LISTING; + opts.listing ? "yes" : "no"; s_http_port = opts.port ? opts.port : MG_PORT; s_http_server_opts.global_auth_file = opts.global_auth_file; s_http_server_opts.auth_domain = opts.auth_domain;