diff mbox series

[Openvpn-devel] Convert cc_check_return to switch/case

Message ID 20200717112953.6699-1-arne@rfc2549.org
State Not Applicable
Headers
Series [Openvpn-devel] Convert cc_check_return to switch/case |

Commit Message

Arne Schwabe July 17, 2020, 1:29 a.m. UTC 
  The return false/return true is the result of
running uncrustify.

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

Comments

David Sommerseth July 17, 2020, 2:06 a.m. UTC | #1 
On 17/07/2020 13:29, Arne Schwabe wrote:
> The return false/return true is the result of
> running uncrustify.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/multi.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 97b7df16..1fdf6ce5 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -2229,22 +2229,16 @@ static inline bool
>  cc_check_return(int *cc_succeeded_count,
>                  enum client_connect_return ret)
>  {
> -    if (ret == CC_RET_SUCCEEDED)
> +    switch (ret)
>      {
> -        (*cc_succeeded_count)++;
> -        return true;
> -    }
> -    else if (ret == CC_RET_FAILED)
> -    {
> -        return false;
> -    }
> -    else if (ret == CC_RET_SKIPPED)
> -    {
> -        return true;
> -    }
> -    else
> -    {
> -        ASSERT(0);
> +        case CC_RET_SUCCEEDED: (*cc_succeeded_count)++;
> +            return true;
> +
> +        case CC_RET_FAILED: return false;
> +
> +        case CC_RET_SKIPPED: return true;
> +
> +        default: ASSERT(0);

Code style police .... Even though it is not clearly defined, but based on the
example here
<https://community.openvpn.net/openvpn/wiki/CodeStyle#Casesinaswitchstatementshouldbreakorexplicitlyfallthrough>
...

... it should be more like:

   switch (ret)
   {
        case CC_RET_SUCCEEDED:
            (*cc_succeeded_count)++;
            return true;

        case CC_RET_FAILED:
            return false;

        case CC_RET_SKIPPED:
            return true;

        default:
            ASSERT(0);
   }


I generally find this approach more readable.
Gert Doering July 19, 2020, 8:51 a.m. UTC | #2 
Hi,

On Fri, Jul 17, 2020 at 02:06:02PM +0200, David Sommerseth wrote:
> On 17/07/2020 13:29, Arne Schwabe wrote:
> > The return false/return true is the result of
> > running uncrustify.
[..]
> > +        case CC_RET_SKIPPED: return true;
> > +
> > +        default: ASSERT(0);
> 
> Code style police .... 

Not needed :-) - that code escaped, and the change-to-switch-case has
long been merged in proper form.

(For the record, this was discovered and discussed on IRC like "5 minutes
after the mail was sent").

gert
diff mbox series

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 97b7df16..1fdf6ce5 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2229,22 +2229,16 @@  static inline bool
 cc_check_return(int *cc_succeeded_count,
                 enum client_connect_return ret)
 {
-    if (ret == CC_RET_SUCCEEDED)
+    switch (ret)
     {
-        (*cc_succeeded_count)++;
-        return true;
-    }
-    else if (ret == CC_RET_FAILED)
-    {
-        return false;
-    }
-    else if (ret == CC_RET_SKIPPED)
-    {
-        return true;
-    }
-    else
-    {
-        ASSERT(0);
+        case CC_RET_SUCCEEDED: (*cc_succeeded_count)++;
+            return true;
+
+        case CC_RET_FAILED: return false;
+
+        case CC_RET_SKIPPED: return true;
+
+        default: ASSERT(0);
     }
 }