>> + int script_ret = openvpn_run_script(&argv, session->opt->es, >> S_EXITCODE, >> + "--auth-user-pass-verify"); > > Perhaps move the retval declaration down here, as we're touching it > anyhow? This switch() is the first place we use it, unless I'm > overlooking something.
You are overlooking the 'goto done' code path for error handling. > >> @@ -1499,7 +1534,11 @@ verify_user_pass(struct user_pass *up, struct >> tls_multi *multi, >> #ifdef PLUGIN_DEF_AUTH >> || s1 == OPENVPN_PLUGIN_FUNC_DEFERRED >> #endif >> - ) && s2 >> + ) && >> + ((s2 == OPENVPN_PLUGIN_FUNC_SUCCESS) >> +#ifdef ENABLE_DEF_AUTH >> + || s2 == OPENVPN_PLUGIN_FUNC_DEFERRED) >> +#endif >> #ifdef MANAGEMENT_DEF_AUTH >> && man_def_auth != KMDA_ERROR >> #endif > > Yikes! This if() statement is unreadable. Since you are doing changes > here, could we improve this whole logic. Perhaps using a helper macro > like this (incorrect code-style, for e-mail readability) > This is already the improved version after the connect patches cleaned it up a bit :P I really don't like hiding condition behind macros just make them shorter, I will propose a variant that extracts parts of the condition into individual booleans. Arne _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
