On Wed, Oct 26, 2016 at 02:29:19PM +0200, Steffan Karger wrote: > Hi, > > On 26-10-16 14:06, Antonio Quartulli wrote: > > clr-verify can be specified multiple times in the config file and the > > expected behaviour is that the last occurrence should be used. > > > > Therefore, reset the optional flags everytime a new crl-verify > > option is found. > > > > Signed-off-by: Antonio Quartulli <[email protected]> > > --- > > src/openvpn/options.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > > index 281ef0b..5192198 100644 > > --- a/src/openvpn/options.c > > +++ b/src/openvpn/options.c > > @@ -6966,6 +6966,14 @@ add_option (struct options *options, > > || (p[2] && streq (p[1], INLINE_FILE_TAG) ) || !p[2]) && > > !p[3]) > > { > > VERIFY_PERMISSION (OPT_P_GENERAL); > > + /* > > + * If crl-verify appears more than once in the config file, we have > > to > > + * to keep settings belonging to the last occurrence only. > > + * Reset optional settings each time. > > + */ > > + options->ssl_flags &= ~SSLF_CRL_VERIFY_DIR; > > + options->crl_file_inline = NULL; > > + > > if (p[2] && streq(p[2], "dir")) > > options->ssl_flags |= SSLF_CRL_VERIFY_DIR; > > options->crl_file = p[1]; > > Feature-ACK. I think this is a great idea. This allows us to do an > if(inline_file), instead of doing a strcmp against the 'magic inline > string' everywhere. > > However, I think it would be better to keep all inline file handling > consistent. So I would prefer a patch that does this for all inline > options, and replaces the strcmp()s at various places with if(inline). > > (Thinking about refactoring this, wouldn't it be even prettier to change > crl_file_inline to a bool, indicating whether crl_file contains a file > path or the file contents? Might be a matter of style though, so feel > free to ignore if you disagree.)
I totally agree on this. I was planning to do the rest of the refactoring in a second patch. At this point I think it would make sense to reject this patch and wait for a full patchset set where I will: 1) "fix" every inline option to be sure that we don't carry around bogus settings 2) switch any *_inline field to bool 3) refactor the code to avoid strcmp() Thanks for the quick reply! Cheers, -- Antonio Quartulli
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
