Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls: factor out mbedtls_x509_get_subject_alt_name_ext usage #511

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

rcmcdonald91
Copy link

Ref: #510

Copy link
Collaborator

@Roytak Roytak left a comment

Choose a reason for hiding this comment

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

The changes look good and are appreciated, however, there are some problems due to the library's coding style and some other small nits. I could also imagine creating a new function that parses the GeneralNames separately instead of it all being inside a single one.

src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
src/session_mbedtls.c Outdated Show resolved Hide resolved
@Roytak
Copy link
Collaborator

Roytak commented Sep 27, 2024

Could you also change the target branch from master to devel, please?

@Roytak
Copy link
Collaborator

Roytak commented Oct 2, 2024

The changes look good, one last thing would be to run make format, because the format test is failing. Will merge once you've run the command and committed the formatted code. Thank you.

@rcmcdonald91
Copy link
Author

The changes look good, one last thing would be to run make format, because the format test is failing. Will merge once you've run the command and committed the formatted code. Thank you.

Done! Thanks

@Roytak Roytak merged commit f1c6d23 into CESNET:devel Oct 4, 2024
15 checks passed
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.

2 participants