Skip to content

dnscache/conncache tweaks - #22900

Closed
icing wants to merge 3 commits into
curl:masterfrom
icing:dnscache-tweaks
Closed

icing wants to merge 3 commits into
curl:masterfrom
icing:dnscache-tweaks

Conversation

@icing

@icing icing commented Sep 10, 2026

Copy link
Copy Markdown
Contributor
  • pass more struct curltime pointers around
  • as hash keys are now bytes, make the dnscache entry key binary as well, no longer using printf() for formatting
  • restrict hostname entry lengths to UINT16_MAX and fail otherwise
  • add a permanent bit to entries

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Long hostnames can alias cache entries, and negative HTTPS entries no longer receive accelerated expiration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refactors DNS cache keys and timestamps while sharing a single current-time value with connection-cache cleanup.

Changes:

  • Uses binary DNS cache keys and explicit permanence state.
  • Records cache insertion times and passes time pointers through pruning.
  • Updates connection-idle timing and related unit tests.
File summaries
File Description
tests/unit/unit1607.c Adapts DNS cache tests to binary keys and permanence.
lib/vdns/dnscache.h Updates DNS entry fields and pruning API.
lib/vdns/dnscache.c Implements binary keys, permanence, and insertion timestamps.
lib/multi.c Shares one timestamp across cleanup operations.
lib/conncache.h Extends the idle-connection API with a timestamp.
lib/conncache.c Uses the caller-provided timestamp.
Review details

Suppressed comments (1)

tests/unit/unit1607.c:220

  • The assertion now checks dns->permanent, but its failure text still refers to a zero timestamp. Report the permanent flag so the diagnostic matches the failed condition.
      if(dns->permanent && !tests[i].permanent) {
  • Files reviewed: 6/6 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/vdns/dnscache.c
Comment thread lib/vdns/dnscache.c Outdated
Comment thread tests/unit/unit1607.c
- pass more `struct curltime` pointers around
- as hash keys are now bytes, make the dnscache entry key
  binary as well, no longer using printf() for formatting
- restrict hostname entry lengths to UINT16_MAX and fail otherwise
- add a `permanent` bit to entries
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.

2 participants