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

Remove rustls_result return value from rustls_server_config_builder_set_persistence #229

Open
jsha opened this issue Nov 14, 2021 · 1 comment · May be fixed by #516
Open

Remove rustls_result return value from rustls_server_config_builder_set_persistence #229

jsha opened this issue Nov 14, 2021 · 1 comment · May be fixed by #516
Assignees

Comments

@jsha
Copy link
Collaborator

jsha commented Nov 14, 2021

This function is currently:

rustls-ffi/src/server.rs

Lines 654 to 675 in 78736fb

pub extern "C" fn rustls_server_config_builder_set_persistence(
builder: *mut rustls_server_config_builder,
get_cb: rustls_session_store_get_callback,
put_cb: rustls_session_store_put_callback,
) -> rustls_result {
ffi_panic_boundary! {
let get_cb: SessionStoreGetCallback = match get_cb {
Some(cb) => cb,
None => return rustls_result::NullParameter,
};
let put_cb: SessionStorePutCallback = match put_cb {
Some(cb) => cb,
None => return rustls_result::NullParameter,
};
let builder: &mut ServerConfigBuilder = try_mut_from_ptr!(builder);
builder.session_storage = Some(Arc::new(SessionStoreBroker::new(
get_cb, put_cb
)));
rustls_result::Ok
}
}
}

Note that the only possible error return cases are that the inputs were NULL. Our API guidelines state that in such a case, we don't bother to return a rustls_result (and make the user check it), but instead do nothing and return void.

@cpu
Copy link
Member

cpu commented Mar 31, 2024

This looks easy to fix, but I'm not sure it's worth breaking semver for. In the primary rustls repo we have a next-major-release label for this. I've created the same here and applied it to this issue so we can think about knocking it out with the next big rustls update.

@cpu cpu self-assigned this Dec 23, 2024
@cpu cpu linked a pull request Dec 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants