diff mbox series

[Openvpn-devel] Explain structvar usage in sample defer plugin.

Message ID 1612163389-16421-1-git-send-email-gcox@mozilla.com
State Accepted
Headers
Series [Openvpn-devel] Explain structvar usage in sample defer plugin. |

Commit Message

Greg Cox Jan. 31, 2021, 8:09 p.m. UTC 
  sample-plugins/defer/simple.c uses OPENVPN_PLUGINv3_STRUCTVER settings
that may not be obvious to a new author.  Add a comment to reduce
possible confusion.
---
 sample/sample-plugins/defer/simple.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

David Sommerseth Feb. 2, 2021, 11:15 p.m. UTC | #1 
On 01/02/2021 08:09, Greg Cox wrote:
> sample-plugins/defer/simple.c uses OPENVPN_PLUGINv3_STRUCTVER settings
> that may not be obvious to a new author.  Add a comment to reduce
> possible confusion.
> ---
>   sample/sample-plugins/defer/simple.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/sample/sample-plugins/defer/simple.c b/sample/sample-plugins/defer/simple.c
> index 05bfc4efa6d85bb1696b6811428260f4cc5e9914..6f08bedd6319827c05786303f8a4007791042208 100644
> --- a/sample/sample-plugins/defer/simple.c
> +++ b/sample/sample-plugins/defer/simple.c
> @@ -69,12 +69,18 @@ static plugin_log_t plugin_log = NULL;
>    * Constants indicating minimum API and struct versions by the functions
>    * in this plugin.  Consult openvpn-plugin.h, look for:
>    * OPENVPN_PLUGIN_VERSION and OPENVPN_PLUGINv3_STRUCTVER
> + *
> + * Strictly speaking, this sample code only requires plugin_log, a feature
> + * of structver version 1.  However, '1' lines up with ancient versions
> + * of openvpn that are past end-of-support.  As such, we are requiring
> + * structver '5' here to indicate a desire for modern openvpn, rather
> + * than a need for any particular feature found in structver beyond '1'.
>    */
>   #define OPENVPN_PLUGIN_VERSION_MIN 3
>   #define OPENVPN_PLUGIN_STRUCTVER_MIN 5
>   
>   /*
> -* Our context, where we keep our state.
> + * Our context, where we keep our state.
>    */
>   
>   struct plugin_context {
> @@ -160,7 +166,6 @@ openvpn_plugin_open_v3(const int v3structver,
>       const char **envp = args->envp;       /* environment variables */
>       struct plugin_context *context;
>   
> -    /* Check API compatibility -- struct version 5 or higher needed */
>       if (v3structver < OPENVPN_PLUGIN_STRUCTVER_MIN)
>       {
>           fprintf(stderr, "%s: this plugin is incompatible with the running version of OpenVPN\n", MODULE);
> @@ -442,7 +447,6 @@ openvpn_plugin_func_v3(const int v3structver,
>                          struct openvpn_plugin_args_func_in const *args,
>                          struct openvpn_plugin_args_func_return *ret)
>   {
> -    /* Check API compatibility -- struct version 5 or higher needed */
>       if (v3structver < OPENVPN_PLUGIN_STRUCTVER_MIN)
>       {
>           fprintf(stderr, "%s: this plugin is incompatible with the running version of OpenVPN\n", MODULE);
> 

Thanks again!  Since this is purely comment fixes, I've only glared at 
these changes, and they look good.

Acked-By: David Sommerseth <davids@openvpn.net>
Gert Doering Feb. 2, 2021, 11:27 p.m. UTC | #2 
Your patch has been applied to the master, release/2.5 and release/2.4 branch.

I have taken the liberty of changing "structvar" to "structver" in the
Subject:

(As a side note, I do not think the structver check in openvpn_plugin_func_v3()
is really necessary - if "open_v3" succeeds, we already know which struct ver
openvpn uses...)

commit fdfbd4441c2225dc69431c57d18291e103c466cf (master)
commit 1f61f3f755a84ed9765da744c7b61a35f36c4d4b (release/2.5)
commit 5b17607ad593bc081dc35b2ed1d6b852786a0d23 (release/2.4)
Author: Greg Cox
Date:   Mon Feb 1 07:09:49 2021 +0000

     Explain structver usage in sample defer plugin.

     Acked-by: David Sommerseth <davids@openvpn.net>
     Message-Id: <1612163389-16421-1-git-send-email-gcox@mozilla.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21540.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/sample/sample-plugins/defer/simple.c b/sample/sample-plugins/defer/simple.c
index 05bfc4efa6d85bb1696b6811428260f4cc5e9914..6f08bedd6319827c05786303f8a4007791042208 100644
--- a/sample/sample-plugins/defer/simple.c
+++ b/sample/sample-plugins/defer/simple.c
@@ -69,12 +69,18 @@  static plugin_log_t plugin_log = NULL;
  * Constants indicating minimum API and struct versions by the functions
  * in this plugin.  Consult openvpn-plugin.h, look for:
  * OPENVPN_PLUGIN_VERSION and OPENVPN_PLUGINv3_STRUCTVER
+ *
+ * Strictly speaking, this sample code only requires plugin_log, a feature
+ * of structver version 1.  However, '1' lines up with ancient versions
+ * of openvpn that are past end-of-support.  As such, we are requiring
+ * structver '5' here to indicate a desire for modern openvpn, rather
+ * than a need for any particular feature found in structver beyond '1'.
  */
 #define OPENVPN_PLUGIN_VERSION_MIN 3
 #define OPENVPN_PLUGIN_STRUCTVER_MIN 5
 
 /*
-* Our context, where we keep our state.
+ * Our context, where we keep our state.
  */
 
 struct plugin_context {
@@ -160,7 +166,6 @@  openvpn_plugin_open_v3(const int v3structver,
     const char **envp = args->envp;       /* environment variables */
     struct plugin_context *context;
 
-    /* Check API compatibility -- struct version 5 or higher needed */
     if (v3structver < OPENVPN_PLUGIN_STRUCTVER_MIN)
     {
         fprintf(stderr, "%s: this plugin is incompatible with the running version of OpenVPN\n", MODULE);
@@ -442,7 +447,6 @@  openvpn_plugin_func_v3(const int v3structver,
                        struct openvpn_plugin_args_func_in const *args,
                        struct openvpn_plugin_args_func_return *ret)
 {
-    /* Check API compatibility -- struct version 5 or higher needed */
     if (v3structver < OPENVPN_PLUGIN_STRUCTVER_MIN)
     {
         fprintf(stderr, "%s: this plugin is incompatible with the running version of OpenVPN\n", MODULE);