Repository navigation
Conversation
ssh_force_knownhost_key_type() called libssh2_knownhost_checkp() once per hashed known_hosts entry, and each call itself rescans the whole known_hosts set. Cache key types already confirmed absent for the connecting host so repeat entries of the same type do not each trigger another full scan. checkp() NOTFOUND for a given (host, port, type) is independent of the key bytes passed in, so this is safe. Behavior for MATCH is unchanged, and only store->typemask is read after a match, which is identical for any entry of the same type. Tests: runtests.pl 3500 1459 600, full SFTP/SCP keyword set (58/58), checksrc.pl.
There was a problem hiding this comment.
🟡 Changes recommended
The six-entry cache does not cover every libssh2 key-type value, leaving some repeated scans unoptimized.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Caches absent SSH host-key types to avoid redundant libssh2_knownhost_checkp() scans.
Changes:
- Tracks key types returning
NOTFOUND. - Skips repeated checks for cached types.
File summaries
| File | Description |
|---|---|
lib/vssh/libssh2.c |
Adds absent key-type caching during known-host lookup. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The earlier cache used a 6-slot append list, but libssh2 exposes more key types than that, so some entry orderings filled it before a type was seen and kept rescanning that type. Use one slot per possible key type, indexed by the type shifted down, so no ordering can overflow it. Drops the linear scan too.
bagder
approved these changes
Sep 11, 2026
Member
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
follow-up to #22874 (landed in 323c184). that fix added a
libssh2_knownhost_checkp()call per hashed known_hosts entry so the entry is matched against the requested host before its key type gets used. eachcheckp()rescans the whole known_hosts set though, so a file with many hashed entries of the same type does one full scan per entry.this caches the key types already confirmed absent for the connecting host.
checkp()returning NOTFOUND for a (host, port, type) means there is no entry of that type for the host at all, which does not depend on the key bytes passed in. so once a type comes back NOTFOUND, any later hashed entry of that same type can be skipped. if the host does have an entry of that type,checkp()returns MATCH or MISMATCH instead and nothing is cached, so real matches are still found. the MATCH path and the switch onstore->typemaskafterward are unchanged.pure perf, no behavior change.
tested: runtests.pl 3500 1459 600, full SFTP/SCP keyword set (58/58), checksrc.pl.