[Openvpn-devel,v5,10/14] client-connect: Move adding inotify watch into its own function

Message ID 20200711093655.23686-10-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v5,01/14] Allow changing fallback cipher from ccd files/client-connect | expand

Commit Message

Arne Schwabe July 10, 2020, 11:36 p.m. UTC
This make the code a bit better readable and also prepares resuing
the function for client-connect return files

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/multi.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

Comments

tincanteksup July 13, 2020, 3:16 a.m. UTC | #1
On 11/07/2020 10:36, Arne Schwabe wrote:
> This make the code a bit better readable and also prepares resuing

resuing -> reusing

(Don't ask me why this is not re-using, which is how I would probably 
spell it and my teacher would laugh at me)

Grammar:
This make the code more readable


> the function for client-connect return files
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>   src/openvpn/multi.c | 43 ++++++++++++++++++++++++++++---------------
>   1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 271d09d8..dafc85f1 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -2828,6 +2828,32 @@ multi_schedule_context_wakeup(struct multi_context *m, struct multi_instance *mi
>                          compute_wakeup_sigma(&mi->context.c2.timeval));
>   }
>   
> +#if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
> +static void
> +add_inotify_file_watch(struct multi_context *m, struct multi_instance *mi,
> +                       int inotify_fd, const char *file)
> +{
> +    /* watch acf file */
> +    long watch_descriptor = inotify_add_watch(inotify_fd, file,
> +                                              IN_CLOSE_WRITE | IN_ONESHOT);
> +    if (watch_descriptor >= 0)
> +    {
> +        if (mi->inotify_watch != -1)
> +        {
> +            hash_remove(m->inotify_watchers,
> +                        (void *) (unsigned long)mi->inotify_watch);
> +        }
> +        hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor,
> +                 mi, true);
> +        mi->inotify_watch = watch_descriptor;
> +    }
> +    else
> +    {
> +        msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
> +    }
> +}
> +#endif /* if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH) */
> +
>   /*
>    * Figure instance-specific timers, convert
>    * earliest to absolute time in mi->wakeup,
> @@ -2865,21 +2891,8 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
>           if (ks && ks->auth_control_file && was_unauthenticated
>               && (ks->authenticated == KS_AUTH_DEFERRED))
>           {
> -            /* watch acf file */
> -            long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
> -            if (watch_descriptor >= 0)
> -            {
> -                if (mi->inotify_watch != -1)
> -                {
> -                    hash_remove(m->inotify_watchers, (void *) (unsigned long)mi->inotify_watch);
> -                }
> -                hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor, mi, true);
> -                mi->inotify_watch = watch_descriptor;
> -            }
> -            else
> -            {
> -                msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
> -            }
> +            add_inotify_file_watch(m, mi, m->top.c2.inotify_fd,
> +                                   ks->auth_control_file);
>           }
>   #endif
>   
>
Antonio Quartulli July 14, 2020, 10:26 p.m. UTC | #2
Hi,

On 11/07/2020 11:36, Arne Schwabe wrote:
> This make the code a bit better readable and also prepares resuing
> the function for client-connect return files
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>


This patch looks good and does what it says. No functional change is
implemented, but it's all about moving a chunk of code in its own
function and indenting it better than how it was.

Acked-by: Antonio Quartulli <antonio@openvpn.net>
Gert Doering July 15, 2020, 3:48 a.m. UTC | #3
Your patch has been applied to the master branch.

(I have merged this out of sequence while we still haggle about
the least ugly way for 08 :-) - it's really "just moving this
code parts", but due to the reformatting and variable renaming
git can't see it.  Compared manually.  Also applied the typo
and grammar changes from tincantech to the commit message)

commit 708d1694c7878c6c538c4ab0961eaaa36c007fd0
Author: Arne Schwabe
Date:   Sat Jul 11 11:36:51 2020 +0200

     client-connect: Move adding inotify watch into its own function

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20200711093655.23686-10-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20284.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 271d09d8..dafc85f1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2828,6 +2828,32 @@  multi_schedule_context_wakeup(struct multi_context *m, struct multi_instance *mi
                        compute_wakeup_sigma(&mi->context.c2.timeval));
 }
 
+#if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
+static void
+add_inotify_file_watch(struct multi_context *m, struct multi_instance *mi,
+                       int inotify_fd, const char *file)
+{
+    /* watch acf file */
+    long watch_descriptor = inotify_add_watch(inotify_fd, file,
+                                              IN_CLOSE_WRITE | IN_ONESHOT);
+    if (watch_descriptor >= 0)
+    {
+        if (mi->inotify_watch != -1)
+        {
+            hash_remove(m->inotify_watchers,
+                        (void *) (unsigned long)mi->inotify_watch);
+        }
+        hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor,
+                 mi, true);
+        mi->inotify_watch = watch_descriptor;
+    }
+    else
+    {
+        msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
+    }
+}
+#endif /* if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH) */
+
 /*
  * Figure instance-specific timers, convert
  * earliest to absolute time in mi->wakeup,
@@ -2865,21 +2891,8 @@  multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
         if (ks && ks->auth_control_file && was_unauthenticated
             && (ks->authenticated == KS_AUTH_DEFERRED))
         {
-            /* watch acf file */
-            long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
-            if (watch_descriptor >= 0)
-            {
-                if (mi->inotify_watch != -1)
-                {
-                    hash_remove(m->inotify_watchers, (void *) (unsigned long)mi->inotify_watch);
-                }
-                hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor, mi, true);
-                mi->inotify_watch = watch_descriptor;
-            }
-            else
-            {
-                msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
-            }
+            add_inotify_file_watch(m, mi, m->top.c2.inotify_fd,
+                                   ks->auth_control_file);
         }
 #endif