Skip to content

HTTP/2, header conversion tightening#12097

Closed
icing wants to merge 3 commits into
curl:masterfrom
icing:dynhds2nva
Closed

HTTP/2, header conversion tightening#12097
icing wants to merge 3 commits into
curl:masterfrom
icing:dynhds2nva

Conversation

@icing

@icing icing commented Oct 12, 2023

Copy link
Copy Markdown
Contributor
  • fold the code to convert dynhds to the nghttp2 structs into a dynhds internal method
  • saves code duplication
  • pacifies compiler analyzers

@icing icing requested review from jay and vszakats October 12, 2023 08:17
@icing icing added the tidy-up label Oct 12, 2023
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 12, 2023
@vszakats

vszakats commented Oct 12, 2023

Copy link
Copy Markdown
Member

Warnings reduced to a single block after the patch.

Before:
https://github.com/curl/curl-for-win/actions/runs/6499470079/job/17652877625#step:3:5316

After:
https://github.com/curl/curl-for-win/actions/runs/6499706712/job/17655051866#step:3:5324

/Users/runner/work/curl-for-win/curl-for-win/curl/lib/vquic/curl_ngtcp2.c:1709:38: warning: potential null pointer dereference [-Wnull-dereference]
 1709 |     nva[i].value = (unsigned char *)e->value;
      |                                     ~^~~~~~~
/Users/runner/work/curl-for-win/curl-for-win/curl/lib/vquic/curl_ngtcp2.c:1708:23: warning: potential null pointer dereference [-Wnull-dereference]
 1708 |     nva[i].namelen = e->namelen;
      |                      ~^~~~~~~~~
/Users/runner/work/curl-for-win/curl-for-win/curl/lib/vquic/curl_ngtcp2.c:1707:37: warning: potential null pointer dereference [-Wnull-dereference]
 1707 |     nva[i].name = (unsigned char *)e->name;
      |                                    ~^~~~~~
/Users/runner/work/curl-for-win/curl-for-win/curl/lib/vquic/curl_ngtcp2.c:1710:24: warning: potential null pointer dereference [-Wnull-dereference]
 1710 |     nva[i].valuelen = e->valuelen;
      |                       ~^~~~~~~~~~

@jay

jay commented Oct 13, 2023

Copy link
Copy Markdown
Member

That's also true of msh3 and quiche. I think the pattern could be eliminated from all of vquic. Otherwise it looks fine.

Comment thread lib/dynhds.c Outdated
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 16, 2023
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 17, 2023
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 18, 2023
icing added 3 commits October 20, 2023 17:25
- fold the code to convert dynhds to the nghttp2 structs
  into a dynhds internal method
- saves code duplication
- pacifies compiler analyzers
@bagder bagder closed this in 117c9bd Oct 21, 2023
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants