[PATCH] CLEANUP: h2: Help static analyzers understand the list's end marker

[Tim Duesterhus]

Willy,

I know you dislike adjusting code to please static analyzers, but I'd argue
that using the new IST_NULL + isttest() combination is easier to understand 
for humans as well. A simple .ptr == NULL check might also be slightly faster
compared to isteq() with an empty string?

I have verified that reg-tests pass, but as this is deep within the internals
please check this carefully.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Clang Static Analyzer (scan-build) was having a hard time to understand that
`hpack_encode_header` would never be called with a garbage value for `v`.

It failed to detect that `if (isteq(n, ist(""))) break;` would exist the
loop in all cases. By setting `n` to `IST_NULL` and checking with
`isttest()` it no longer complains.

The check must be moved to the beginning of the loop to prevent a NULL
pointer derefence for the pseudo header skip.
---
 src/mux_h2.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 013ef86f8..b353c4d7c 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -4617,7 +4617,7 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s, 
struct htx *htx)
        }
 
        /* marker for end of headers */
-       list[hdr].n = ist("");
+       list[hdr].n = IST_NULL;
 
        if (h2s->status == 204 || h2s->status == 304) {
                /* no contents, claim c-len is present and set to zero */
@@ -4660,6 +4660,9 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s, 
struct htx *htx)
 
        /* encode all headers, stop at empty name */
        for (hdr = 0; hdr < sizeof(list)/sizeof(list[0]); hdr++) {
+               if (!isttest(list[hdr].n))
+                       break; // end
+
                /* these ones do not exist in H2 and must be dropped. */
                if (isteq(list[hdr].n, ist("connection")) ||
                    isteq(list[hdr].n, ist("proxy-connection")) ||
@@ -4672,9 +4675,6 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s, 
struct htx *htx)
                if (*(list[hdr].n.ptr) == ':')
                        continue;
 
-               if (isteq(list[hdr].n, ist("")))
-                       break; // end
-
                if (!hpack_encode_header(&outbuf, list[hdr].n, list[hdr].v)) {
                        /* output full */
                        if (b_space_wraps(mbuf))
@@ -4870,7 +4870,7 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s, 
struct htx *htx)
        }
 
        /* marker for end of headers */
-       list[hdr].n = ist("");
+       list[hdr].n = IST_NULL;
 
        mbuf = br_tail(h2c->mbuf);
  retry:
@@ -5007,6 +5007,9 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s, 
struct htx *htx)
                struct ist n = list[hdr].n;
                struct ist v = list[hdr].v;
 
+               if (!isttest(n))
+                       break; // end
+
                /* these ones do not exist in H2 and must be dropped. */
                if (isteq(n, ist("connection")) ||
                    (auth.len && isteq(n, ist("host"))) ||
@@ -5030,9 +5033,6 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s, 
struct htx *htx)
                if (*(n.ptr) == ':')
                        continue;
 
-               if (isteq(n, ist("")))
-                       break; // end
-
                if (!hpack_encode_header(&outbuf, n, v)) {
                        /* output full */
                        if (b_space_wraps(mbuf))
-- 
2.25.2