Re: [Openvpn-devel] [PATCH v5 06/14] client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop
Hi,
On 11/07/2020 11:36, Arne Schwabe wrote:
> From: Fabian Knittel
>
> This patch changes the calling of the client-connect functions into an array
> of hooks and a block of code that calls them in a loop.
>
> Signed-off-by: Fabian Knittel
> Signed-off-by: Arne Schwabe
>
> Patch V5: Rebase on master.
>
> Signed-off-by: Arne Schwabe
> ---
> src/openvpn/multi.c | 43 +--
> 1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 9bb52ef7..83848fdc 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -2241,6 +2241,10 @@ cc_check_return(int *cc_succeeded_count,
> }
> }
>
> +typedef enum client_connect_return (*multi_client_connect_handler)
> +(struct multi_context *m, struct multi_instance *mi,
> + unsigned int *option_types_found);
> +
how about something like this:
2519 typedef enum client_connect_return
2520 (*multi_client_connect_handler)(struct multi_context *m,
2521 struct multi_instance *mi,
2522 unsigned int *option_types_found);
2523
I find it easier to read (just my opinion)
> /*
> * Called as soon as the SSL/TLS connection is authenticated.
> *
> @@ -2268,7 +2272,17 @@ multi_connection_established(struct multi_context *m,
> struct multi_instance *mi)
> {
> return;
> }
> -unsigned int option_types_found = 0;
> +
> +multi_client_connect_handler handlers[] = {
> +multi_client_connect_source_ccd,
> +multi_client_connect_call_plugin_v1,
> +multi_client_connect_call_plugin_v2,
> +multi_client_connect_call_script,
> +multi_client_connect_mda,
> +NULL
> +};
> +
> +unsigned int option_types_found = 0;
something is wrong in the indentation of the chunk above: too many spaces.
>
> int cc_succeeded = true; /* client connect script status */
> int cc_succeeded_count = 0;
> @@ -2276,32 +2290,9 @@ multi_connection_established(struct multi_context *m,
> struct multi_instance *mi)
>
> multi_client_connect_early_setup(m, mi);
>
> -ret = multi_client_connect_source_ccd(m, mi, &option_types_found);
> -cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
> -
> -if (cc_succeeded)
> -{
> -ret = multi_client_connect_call_plugin_v1(m, mi,
> &option_types_found);
> -cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
> -}
> -
> -if (cc_succeeded)
> -{
> -ret = multi_client_connect_call_plugin_v2(m, mi,
> &option_types_found);
> -cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
> -}
> -
> -
> -if (cc_succeeded)
> +for (int i = 0; cc_succeeded && handlers[i]; i++)
> {
> -ret = multi_client_connect_call_script(m, mi, &option_types_found);
> -cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
> -}
> -
> -if (cc_succeeded)
> -{
> -
> -ret = multi_client_connect_mda(m, mi, &option_types_found);
> +ret = handlers[i](m, mi, &option_types_found);
> cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
> }
>
>
Except for the indentation issue, the rest looks good. This patch simply
makes the handlers invocation more generic and part of a loop.
Acked-by: Antonio Quartulli
--
Antonio Quartulli
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v5 06/14] client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop
Hi, On Sat, Jul 11, 2020 at 11:36:47AM +0200, Arne Schwabe wrote: > From: Fabian Knittel > > This patch changes the calling of the client-connect functions into an array > of hooks and a block of code that calls them in a loop. > > Signed-off-by: Fabian Knittel > Signed-off-by: Arne Schwabe 23... Test sets succeeded: 1 1a 1b 1d 2 2a 2b 2c 2d 3 4 5 6 8 8a 9. Test sets failed: none. 24... Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 4a 5 6 8 8a 9. Test sets failed: none. master... Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 5 6 7 7a 8 8a 9 2f 4b. Test sets failed: none. I dot not test all variants yet, though, so this is not a sufficient test to say "everything works". Generally speaking the patch looks quite reasonable. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v5 06/14] client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop
From: Fabian Knittel
This patch changes the calling of the client-connect functions into an array
of hooks and a block of code that calls them in a loop.
Signed-off-by: Fabian Knittel
Signed-off-by: Arne Schwabe
Patch V5: Rebase on master.
Signed-off-by: Arne Schwabe
---
src/openvpn/multi.c | 43 +--
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 9bb52ef7..83848fdc 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2241,6 +2241,10 @@ cc_check_return(int *cc_succeeded_count,
}
}
+typedef enum client_connect_return (*multi_client_connect_handler)
+(struct multi_context *m, struct multi_instance *mi,
+ unsigned int *option_types_found);
+
/*
* Called as soon as the SSL/TLS connection is authenticated.
*
@@ -2268,7 +2272,17 @@ multi_connection_established(struct multi_context *m,
struct multi_instance *mi)
{
return;
}
-unsigned int option_types_found = 0;
+
+multi_client_connect_handler handlers[] = {
+multi_client_connect_source_ccd,
+multi_client_connect_call_plugin_v1,
+multi_client_connect_call_plugin_v2,
+multi_client_connect_call_script,
+multi_client_connect_mda,
+NULL
+};
+
+unsigned int option_types_found = 0;
int cc_succeeded = true; /* client connect script status */
int cc_succeeded_count = 0;
@@ -2276,32 +2290,9 @@ multi_connection_established(struct multi_context *m,
struct multi_instance *mi)
multi_client_connect_early_setup(m, mi);
-ret = multi_client_connect_source_ccd(m, mi, &option_types_found);
-cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
-
-if (cc_succeeded)
-{
-ret = multi_client_connect_call_plugin_v1(m, mi, &option_types_found);
-cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
-}
-
-if (cc_succeeded)
-{
-ret = multi_client_connect_call_plugin_v2(m, mi, &option_types_found);
-cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
-}
-
-
-if (cc_succeeded)
+for (int i = 0; cc_succeeded && handlers[i]; i++)
{
-ret = multi_client_connect_call_script(m, mi, &option_types_found);
-cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
-}
-
-if (cc_succeeded)
-{
-
-ret = multi_client_connect_mda(m, mi, &option_types_found);
+ret = handlers[i](m, mi, &option_types_found);
cc_succeeded = cc_check_return(&cc_succeeded_count, ret);
}
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
