Skip to content

Do some tweaks and cleanups #60

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

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Do some tweaks and cleanups #60

merged 2 commits into from
Mar 12, 2021

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Mar 11, 2021

Order imports as: std, third-party, crate.

Use try_ref_from_ptr! to turn out_n's into refs. This moves unsafe blocks from later in the function to earlier in the function, closer to where their safety properties are checked. Also, remove pre-emptive zeroing of out_n's. According to the conventions listed in our README, there are no partial successes or partial failures, so we shouldn't write to out_n if we early-return.

Remove strong_count < 1 checks on Arc. Follow-up from #32.

Add some missing comments and fix some formatting.

@jsha jsha requested a review from tgeoghegan March 11, 2021 19:42
@jsha jsha force-pushed the tweaks-and-cleanups branch from 15776bb to be46156 Compare March 11, 2021 19:42
Some(out_n) => out_n,
None => return,
};
*out_n = 0;
if buf.is_null() {
return;
}
slice::from_raw_parts_mut(buf as *mut u8, len as usize)
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the pattern we were talking about in Slack earlier. How about:

if buf.is_null() {
    return;
}
let write_buf: &mut [u8] = unsafe { slice::from_raw_parts_mut(buf as *mut u8, len as usize) };

Copy link
Member

Choose a reason for hiding this comment

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

(Is the Slack instance accessible to me? Might be useful...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep! Josh sent you an invite. Let me know if you didn't receive it.

src/cipher.rs Outdated
pub struct rustls_certified_key {
// We use the opaque struct pattern to tell C about our types without
// telling them what's inside.
// https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs
_private: [u8; 0],
}

/// Build a `rustls_certified_key` from a certificate chain and a private key.
/// `cert_chain` must point to an array of `cert_chain_len` bytes, containing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// `cert_chain` must point to an array of `cert_chain_len` bytes, containing
/// `cert_chain` must point to a buffer of `cert_chain_len` bytes, containing

Per discussion in Slack earlier today

src/server.rs Outdated
@@ -105,6 +105,16 @@ pub extern "C" fn rustls_server_config_builder_set_ignore_client_order(
}
}

/// Set the ALPN protocol list to the given protocols. `protocols` must point
/// to a buffer of `rustls_slice_bytes` (built by the caller) with `len`
/// elements. Each element of th buffer must point to a slice of bytes that
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// elements. Each element of th buffer must point to a slice of bytes that
/// elements. Each element of the buffer must point to a slice of bytes that

src/cipher.rs Outdated
/// a series of PEM-encoded certificates, with the end-entity (leaf)
/// certificate first.
///
/// `private_key` must point to an array of `private_key_len` bytes, containing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// `private_key` must point to an array of `private_key_len` bytes, containing
/// `private_key` must point to a buffer of `private_key_len` bytes, containing

@jsha jsha merged commit 2a8d471 into main Mar 12, 2021
@jsha jsha deleted the tweaks-and-cleanups branch March 12, 2021 00:56
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