[Openvpn-devel] Convert cc_check_return to switch/case

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

Commit Message

Arne Schwabe July 17, 2020, 11:29 a.m.
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, 12:06 p.m. | #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, 6:51 p.m. | #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

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);
     }
 }