From a07e9819fbf24f998643812566f3ec3cf3a2204d Mon Sep 17 00:00:00 2001
From: Ed Tanous <ed@tanous.net>
Date: Tue, 19 Mar 2024 10:31:13 -0700
Subject: Don't use switch for single conditional http2

This code was copied from one of the nghttp2 examples, and makes things
more complicated than they should be.  We only handle one case here, so
a pattern of returning early is easier.

Also, this resolves a possible clang-tidy bugprone warning (that we
don't yet enable).

Tested: Http2 unit tests pass (good coverage for this case).

Change-Id: Ie8606872f3a96f1bb0329bf22a4f7429f431bbef
Signed-off-by: Ed Tanous <ed@tanous.net>
---
 http/http2_connection.hpp | 78 +++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 40 deletions(-)

(limited to 'http')

diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index a698d9eb0d..7903ec93b1 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -396,49 +396,47 @@ class HTTP2Connection :
 
         BMCWEB_LOG_DEBUG("on_header_callback name: {} value {}", nameSv,
                          valueSv);
-
-        switch (frame.hd.type)
+        if (frame.hd.type != NGHTTP2_HEADERS)
         {
-            case NGHTTP2_HEADERS:
-                if (frame.headers.cat != NGHTTP2_HCAT_REQUEST)
-                {
-                    break;
-                }
-                auto thisStream = streams.find(frame.hd.stream_id);
-                if (thisStream == streams.end())
-                {
-                    BMCWEB_LOG_ERROR("Unknown stream{}", frame.hd.stream_id);
-                    close();
-                    return -1;
-                }
+            return 0;
+        }
+        if (frame.headers.cat != NGHTTP2_HCAT_REQUEST)
+        {
+            return 0;
+        }
+        auto thisStream = streams.find(frame.hd.stream_id);
+        if (thisStream == streams.end())
+        {
+            BMCWEB_LOG_ERROR("Unknown stream{}", frame.hd.stream_id);
+            close();
+            return -1;
+        }
 
-                crow::Request& thisReq = thisStream->second.req;
+        crow::Request& thisReq = thisStream->second.req;
 
-                if (nameSv == ":path")
-                {
-                    thisReq.target(valueSv);
-                }
-                else if (nameSv == ":method")
-                {
-                    boost::beast::http::verb verb =
-                        boost::beast::http::string_to_verb(valueSv);
-                    if (verb == boost::beast::http::verb::unknown)
-                    {
-                        BMCWEB_LOG_ERROR("Unknown http verb {}", valueSv);
-                        close();
-                        return -1;
-                    }
-                    thisReq.req.method(verb);
-                }
-                else if (nameSv == ":scheme")
-                {
-                    // Nothing to check on scheme
-                }
-                else
-                {
-                    thisReq.req.set(nameSv, valueSv);
-                }
-                break;
+        if (nameSv == ":path")
+        {
+            thisReq.target(valueSv);
+        }
+        else if (nameSv == ":method")
+        {
+            boost::beast::http::verb verb =
+                boost::beast::http::string_to_verb(valueSv);
+            if (verb == boost::beast::http::verb::unknown)
+            {
+                BMCWEB_LOG_ERROR("Unknown http verb {}", valueSv);
+                close();
+                return -1;
+            }
+            thisReq.req.method(verb);
+        }
+        else if (nameSv == ":scheme")
+        {
+            // Nothing to check on scheme
+        }
+        else
+        {
+            thisReq.req.set(nameSv, valueSv);
         }
         return 0;
     }
-- 
cgit v1.2.3