Skip to content

refactor parts of DoServerKeyExchange()#9176

Merged
SparkiDev merged 1 commit intowolfSSL:masterfrom
effbiae:do-server-key-exchange
Sep 18, 2025
Merged

refactor parts of DoServerKeyExchange()#9176
SparkiDev merged 1 commit intowolfSSL:masterfrom
effbiae:do-server-key-exchange

Conversation

@effbiae
Copy link
Copy Markdown
Contributor

@effbiae effbiae commented Sep 10, 2025

Description

#9165 aligned parts of DoServerKeyExchange(). now, the common parts of DoServerKeyExchange() are pulled out to separate functions: GetPSKServerHint() and GetEcDiffieHellmanKea(). maybe there are better names for these.

Testing

./configure && make check && ./configure --enable-all && make check

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@wolfSSL-Bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@effbiae effbiae force-pushed the do-server-key-exchange branch from 80d751b to 0d3ea3e Compare September 10, 2025 09:44
@dgarske
Copy link
Copy Markdown
Member

dgarske commented Sep 10, 2025

Okay to test. Contributor agreement on file

@dgarske
Copy link
Copy Markdown
Member

dgarske commented Sep 10, 2025

Jenkins retest this please: Several tests failed with "RequestAbortedException" and it does not appear related to these chagnes.

@dgarske
Copy link
Copy Markdown
Member

dgarske commented Sep 11, 2025

Hi @effbiae can you please rebase and force push to clear those Ubuntu --enable-coding=no issues? This refactor looks good.

@dgarske dgarske removed their assignment Sep 11, 2025
@dgarske dgarske requested a review from SparkiDev September 11, 2025 17:39
Copy link
Copy Markdown
Contributor

@SparkiDev SparkiDev left a comment

Choose a reason for hiding this comment

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

Great refactor!
Cleans the code up nicely!

Comment thread src/internal.c Outdated
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Looks like hard tabs vs 4 spaces in a few places.

./src/internal.c:32271:»return ECC_CURVE_ERROR;
./src/internal.c:32280:»return BUFFER_ERROR;
./src/internal.c:32284:»if (ssl->peerX25519Key == NULL) {
./src/internal.c:32285:»    ret = AllocKey(ssl, DYNAMIC_TYPE_CURVE25519,
./src/internal.c:32286:»                   (void **)&ssl->peerX25519Key);
./src/internal.c:32287:»    if (ret != 0) {
./src/internal.c:32288:»»return ret;
./src/internal.c:32289:»    }
[...and more...]

@effbiae effbiae force-pushed the do-server-key-exchange branch from c57d527 to 0a99c95 Compare September 13, 2025 02:23
@effbiae
Copy link
Copy Markdown
Contributor Author

effbiae commented Sep 13, 2025

oops -- should have been indent -linux -nut -i4

Comment thread src/internal.c Outdated
@effbiae effbiae force-pushed the do-server-key-exchange branch from 0a99c95 to 7da0b54 Compare September 16, 2025 02:03
@effbiae
Copy link
Copy Markdown
Contributor Author

effbiae commented Sep 17, 2025

what happened in PRB-master-job?

@SparkiDev
Copy link
Copy Markdown
Contributor

SparkiDev commented Sep 17, 2025

retest this please

Too old to see what went wrong.
Probably an issue with infrastructure.

@SparkiDev SparkiDev dismissed dgarske’s stale review September 18, 2025 22:35

Issue raised was fixed.

@SparkiDev SparkiDev merged commit b90720c into wolfSSL:master Sep 18, 2025
258 of 259 checks passed
@effbiae effbiae deleted the do-server-key-exchange branch September 24, 2025 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants