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.

Sign up for GitHub

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

Fixes broken link #4060

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Fixes broken link #4060

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c3ff1e8
Fixed link
maddeleine Jun 15, 2023
e1a2611
More accurate link
maddeleine Jun 15, 2023
521d1d3
Merge branch 'main' into broken_link
maddeleine Jun 21, 2023
b89d10c
Merge branch 'main' into broken_link
maddeleine Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bindings/rust/s2n-tls/src/testing/s2n_tls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ impl<'a, T: 'a + Context> Callback<'a, T> {
let data = core::slice::from_raw_parts_mut(data, len as _);
match context.on_read(data) {
0 => {
// https://github.com/awslabs/s2n/blob/main/docs/USAGE-GUIDE.md#s2n_connection_set_send_cb
// https://aws.github.io/s2n-tls/doxygen/s2n_8h.html#a699fd9e05a8e8163811db6cab01af973
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the link to the correct section right now-- will it always be? Is this a predictable hash, or is it random / based on the order of methods in the file?

Copy link
Contributor Author

@maddeleine maddeleine Jun 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

According to this issue the hash won't change unless we change the definition of the function. So this link won't break unless the API changes, which is unlikely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this is overkill, but we could probably get the domain for something like docs.s2n-tls.aws.dev and then CNAME that to the doxygen site. This would also let get more human readable names for the things that host our CloudFront distributions with coverage reports, benchmark reports, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of a hash is it possible to generate nicer human readable names for the url?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not possible to do that with the current doxygen setup. The links are hashes on purpose, as the docs mention, because cpp can overload function names and therefore it wouldn't be clear which function the link was referring to.

// s2n-tls wants us to set the global errno to signal blocked
errno::set_errno(errno::Errno(libc::EWOULDBLOCK));
-1
Expand Down