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

Use Wrapping for intended underflow of unsigned integer value. #369

Merged
merged 1 commit into from
May 6, 2016
Merged

Use Wrapping for intended underflow of unsigned integer value. #369

merged 1 commit into from
May 6, 2016

Conversation

fiveop
Copy link
Contributor

@fiveop fiveop commented May 4, 2016

This fixes an warning made error on nightly builds.

@fiveop
Copy link
Contributor Author

fiveop commented May 4, 2016

r? @kamalmarhubi

@@ -111,11 +111,15 @@ pub fn chdir<P: ?Sized + NixPath>(path: &P) -> Result<()> {
Errno::result(res).map(drop)
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for adding these 2 lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I'll fix it.

@fiveop
Copy link
Contributor Author

fiveop commented May 6, 2016

I readded the (reworded) comment.

use std::num::Wrapping;
unsafe { libc::chown(cstr.as_ptr(),
owner.unwrap_or((Wrapping(0 as uid_t) - Wrapping(1 as uid_t)).0),
group.unwrap_or((Wrapping(0 as gid_t) - Wrapping(1 as gid_t)).0)) }
Copy link
Member

Choose a reason for hiding this comment

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

one last nit, can we use 0.wrapping_sub(1) instead of using Wrapping? I think it'll be easier to read and doesn't require an extra import.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly! If I had known the function, I would have used it right away.

@kamalmarhubi
Copy link
Member

@homu r+ 2139c90

homu added a commit that referenced this pull request May 6, 2016
Use Wrapping for intended underflow of unsigned integer value.

This fixes an warning made error on nightly builds.
@homu
Copy link
Contributor

homu commented May 6, 2016

⌛ Testing commit 2139c90 with merge 7931e48...

@homu
Copy link
Contributor

homu commented May 6, 2016

☀️ Test successful - status

@homu homu merged commit 2139c90 into nix-rust:master May 6, 2016
@fiveop fiveop deleted the fixoverflow branch May 7, 2016 12:21
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.

4 participants