Skip to content

Conversation

@KaganCanSit
Copy link
Contributor

Hello Botan Developer Team,

This pull request contains some TODO message resolves for socket utilities;

  • Replaced inet_pton with string_to_ipv4 for IPv4 validation
    • Since there is a need for an additional parsing functions for IPv6 here, I have not updated ipv_6 for now.
  • Implemented strerror_s per TODO comment - Windows
    • Uses 94-byte buffer as specified in Microsoft documentation
    • Thread-safe error string conversion

I don't except there to be any behavioral difference here. I believe I tested it by compiling with the following command on the Linux but need more additional testing. For Windows I will investigate the situation in CI.

ninja clean && ninja && python3 src/scripts/test_cli.py ./botan cli_tls

Regards.

@coveralls
Copy link

coveralls commented Oct 27, 2025

Coverage Status

coverage: 90.669% (+0.002%) from 90.667%
when pulling e354b41 on KaganCanSit:remove-todos-socket-utilities
into 1fbc54f on randombit:master.

Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

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

Some things regarding C-string handling, otherwise LGTM. Thank you!

@KaganCanSit KaganCanSit force-pushed the remove-todos-socket-utilities branch 3 times, most recently from f2179df to 818b147 Compare October 28, 2025 17:50
@KaganCanSit KaganCanSit requested a review from reneme October 28, 2025 19:06
@KaganCanSit KaganCanSit force-pushed the remove-todos-socket-utilities branch from 818b147 to a3cf170 Compare October 29, 2025 07:48
@KaganCanSit
Copy link
Contributor Author

KaganCanSit commented Oct 29, 2025

Errors are appearing in Windows tests, but I don't think this is related to changes in this branch.

I occasionally see the message “Failure 1: Serpent producer 'avx512' unexpected result for encrypt” in the ‘Nightly’ build and other branches. I can retrigger CI with the “amend” comment without making any changes, or merge if appropriate.
Update: For this case, commit number 2cb6085 was included. I rebased branch.

@KaganCanSit KaganCanSit force-pushed the remove-todos-socket-utilities branch from a3cf170 to e354b41 Compare October 29, 2025 10:01
@KaganCanSit KaganCanSit requested a review from reneme October 29, 2025 11:52
@reneme reneme merged commit 19e477f into randombit:master Oct 29, 2025
44 checks passed
@KaganCanSit KaganCanSit deleted the remove-todos-socket-utilities branch October 29, 2025 12:06
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.

3 participants