Message ID | 20181121101019.1801-3-arne@rfc2549.org |
---|---|
State | Superseded |
Delegated to: | Antonio Quartulli |
Headers | show |
Series | Deferred client-connect patch set | expand |
Hi, On 21/11/2018 11:10, Arne Schwabe wrote: > From: Fabian Knittel <fabian.knittel@lettink.de> > > Refactor multi_client_connect_source_ccd(), so that options_server_import() (or > the success path in general) is only entered in one place within the function. > > Signed-off-by: Fabian Knittel <fabian.knittel@lettink.de> > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/multi.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c > index a4b62151..8fc69a07 100644 > --- a/src/openvpn/multi.c > +++ b/src/openvpn/multi.c > @@ -2063,31 +2063,30 @@ multi_client_connect_source_ccd(struct multi_context *m, > &gc); > > /* try common-name file */ > - if (platform_test_file(ccd_file)) > + if (!platform_test_file(ccd_file)) > { > - options_server_import(&mi->context.options, > - ccd_file, > - D_IMPORT_ERRORS|M_OPTERR, > - CLIENT_CONNECT_OPT_MASK, > - option_types_found, > - mi->context.c2.es); > + ccd_file = NULL; > } > - else /* try default file */ > + /* try default file */ I don't get the change above. You are removing the "else" but now what's the reason for testing "platform_test_file()" and setting ccd_file to NULL if ccd_file is getting re-assigned just below ? it seems like this is a buggy behviaour and it is not matching what was happening before the refactoring. NAK. > { > ccd_file = platform_gen_path(mi->context.options.client_config_dir, > CCD_DEFAULT, > &gc); > - > - if (platform_test_file(ccd_file)) > + if (!platform_test_file(ccd_file)) > { > - options_server_import(&mi->context.options, > - ccd_file, > - D_IMPORT_ERRORS|M_OPTERR, > - CLIENT_CONNECT_OPT_MASK, > - option_types_found, > - mi->context.c2.es); > + ccd_file = NULL; > } > } > + > + if (ccd_file) > + { > + options_server_import(&mi->context.options, > + ccd_file, > + D_IMPORT_ERRORS|M_OPTERR, > + CLIENT_CONNECT_OPT_MASK, > + option_types_found, > + mi->context.c2.es); > + } > gc_free(&gc); > } > } >
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index a4b62151..8fc69a07 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2063,31 +2063,30 @@ multi_client_connect_source_ccd(struct multi_context *m, &gc); /* try common-name file */ - if (platform_test_file(ccd_file)) + if (!platform_test_file(ccd_file)) { - options_server_import(&mi->context.options, - ccd_file, - D_IMPORT_ERRORS|M_OPTERR, - CLIENT_CONNECT_OPT_MASK, - option_types_found, - mi->context.c2.es); + ccd_file = NULL; } - else /* try default file */ + /* try default file */ { ccd_file = platform_gen_path(mi->context.options.client_config_dir, CCD_DEFAULT, &gc); - - if (platform_test_file(ccd_file)) + if (!platform_test_file(ccd_file)) { - options_server_import(&mi->context.options, - ccd_file, - D_IMPORT_ERRORS|M_OPTERR, - CLIENT_CONNECT_OPT_MASK, - option_types_found, - mi->context.c2.es); + ccd_file = NULL; } } + + if (ccd_file) + { + options_server_import(&mi->context.options, + ccd_file, + D_IMPORT_ERRORS|M_OPTERR, + CLIENT_CONNECT_OPT_MASK, + option_types_found, + mi->context.c2.es); + } gc_free(&gc); } }