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

Improve our use of unsafe code #5413

Merged
merged 7 commits into from Sep 11, 2019

Conversation

@nrc
Copy link
Contributor

nrc commented Sep 5, 2019

What have you changed?

I've reduced our unsafe footprint by removing or restricting some uses of unsafe. I've documented some unsafe usage. I've changed some uses of unsafe in minor ways to better reflect the actual unsafeness and make it easier to check the necessary invariants. This is not a full audit of our unsafe code, just a small step.

What is the type of the changes?

  • Engineering (engineering change which doesn't change any feature or fix any issue)

How is the PR tested?

N/A

Does this PR affect documentation (docs) or should it be mentioned in the release notes?

No

Does this PR affect tidb-ansible?

No

PTAL @breeswish @5kbpers

@nrc nrc requested review from breeswish and 5kbpers Sep 5, 2019
@nrc nrc force-pushed the nrc:unsafe branch from 43e0847 to 3d5991b Sep 5, 2019
}

fn split_ymd_hms_with_frac_as_s(
// Safety: caller must ensure `s` and `frac` are valid ascii.

This comment has been minimized.

Copy link
@hicqu

hicqu Sep 5, 2019

Contributor

The comment is also stale. Please also check other places if it's real an issue.

This comment has been minimized.

Copy link
@nrc

nrc Sep 6, 2019

Author Contributor

I've updated the wording on this one.

@siddontang

This comment has been minimized.

Copy link
Contributor

siddontang commented Sep 6, 2019

PTAL @breeswish

@nrc nrc force-pushed the nrc:unsafe branch from 9505ca8 to 0f48280 Sep 6, 2019
Copy link
Contributor

5kbpers left a comment

reset LGTM

@nrc nrc force-pushed the nrc:unsafe branch 2 times, most recently from 8a497b6 to d5937f7 Sep 9, 2019
@nrc

This comment has been minimized.

Copy link
Contributor Author

nrc commented Sep 9, 2019

ping @hicqu are your comments addressed?

@nrc

This comment has been minimized.

Copy link
Contributor Author

nrc commented Sep 11, 2019

/merge

@nrc

This comment has been minimized.

Copy link
Contributor Author

nrc commented Sep 11, 2019

/run-all-tests

@nrc

This comment has been minimized.

Copy link
Contributor Author

nrc commented Sep 11, 2019

/merge

@sre-bot sre-bot added the S: CanMerge label Sep 11, 2019
@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Sep 11, 2019

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Sep 11, 2019

@nrc merge failed.

@nrc nrc dismissed stale reviews from hicqu and brson via 327d175 Sep 11, 2019
@nrc nrc force-pushed the nrc:unsafe branch from 40cb022 to 327d175 Sep 11, 2019
nrc added 3 commits Sep 5, 2019
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
nrc added 4 commits Sep 5, 2019
Signed-off-by: Nick Cameron <nrc@ncameron.org>
The crate is tiny and has no deps, and does a better job of securely zeroing the
memory than we did.

Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Changes
Signed-off-by: Nick Cameron <nrc@ncameron.org>
@nrc nrc force-pushed the nrc:unsafe branch from 327d175 to a7af843 Sep 11, 2019
@brson
brson approved these changes Sep 11, 2019
@brson brson merged commit a556052 into tikv:master Sep 11, 2019
4 checks passed
4 checks passed
DCO All commits are signed off!
Details
idc-jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-cop-push-down-test Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
* Replace as_slice with std::from_ref

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Fixup some docs

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Tidy up profiling code

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Remove erase_lifetime_mut and move erase_lifetime to discourage reuse

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Use the zeroize crate rather than zeroing our self.

The crate is tiny and has no deps, and does a better job of securely zeroing the
memory than we did.

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Document some uses of unsafe code

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Changes

Signed-off-by: Nick Cameron <nrc@ncameron.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.