sectransp: Use common code for cipher suite lookup#13521
sectransp: Use common code for cipher suite lookup#13521jan2000 wants to merge 1 commit intocurl:masterfrom
Conversation
1f7118d to
1ae7131
Compare
lib/vtls/sectransp.c
Outdated
There was a problem hiding this comment.
This big table can be removed (or replaced with something smaller) in a future PR. However, to make sure there is no change in behaviour with this commit it is left in for now.
lib/vtls/sectransp.c
Outdated
There was a problem hiding this comment.
These wrapper functions for Curl_cipher_suite_get_str and Curl_cipher_suite_walk_str are here so there is no change in behavior with this commit. But, I guess no one will shed a tear if support for setting/getting these ciphers is removed?
nickzman
left a comment
There was a problem hiding this comment.
Thanks for your work on this. I only noticed some minor things; the code looks okay, and it doesn’t appear to introduce any obvious regressions.
lib/vtls/sectransp.c
Outdated
There was a problem hiding this comment.
That should be “even” supported. And I have no idea if they’re supported or not.
There was a problem hiding this comment.
Well actually I meant to say "were these ever supported ?", but also in present and future tense. As in: can this be safely removed, or should this be kept in for backward compatibility?
lib/vtls/sectransp.c
Outdated
1ae7131 to
7746df8
Compare
Take advantage of the Curl_cipher_suite_walk_str() and Curl_cipher_suite_get_str() functions introduced in commit fba9afe.
7746df8 to
18ed50c
Compare
|
@nickzman I made the changes you suggested. See https://github.com/curl/curl/compare/1ae71316953f0aa57e5c6a8ad0cb00b9a7deb317..7746df8cf110219407aeeb07e04167d50b336fe9 However, after pushing I saw CI tests failing due to unit2604. So I rebased to latest master and pushed again. Sorry if this caused any inconvenience for the review. But now I am a bit puzzled as to how the test could be failing on unit2604 in the first place, as it was not yet in the master that this PR was initially based on. How does that work? |
nickzman
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your work on this.
|
Thanks! |
Take advantage of the Curl_cipher_suite_walk_str() and Curl_cipher_suite_get_str() functions introduced in commit fba9afe.