Hi Tim,
On Thu, Mar 19, 2020 at 03:15:24PM +0100, Tim Duesterhus wrote:
> 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.
Actually I'm pretty sure that I did it this way precisely for performance
reasons: avoid repeatedly checking a pointer for half of the headers which
are pseudo headers (method, scheme, authority, path just for the request).
It's perfectly possible that the difference is negligible though, but if
it's not, I'm sorry but I'll favor performance over static analysers'
own pleasure. So this one will definitely deserve a test.
Thanks,
Willy
