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 FIXME #2699---I believe the calculation is correct. Fixes #2699. #6053

Closed
wants to merge 1 commit into from

Conversation

nikomatsakis
Copy link
Contributor

r? @jld or @graydon

The calculation looks right to me, but perhaps one of you two can double check. You two seem like you are doing the most recent work in this sort of area.

@thestinger
Copy link
Contributor

It seems like it could be wrong if the header alignment was smaller than the body alignment. However... the header is 4 words so that's pretty unlikely to be true on any architecture.

@nikomatsakis
Copy link
Contributor Author

@thestinger maybe I'm being dense, but why does the alignment of the header matter? Also, the alignment is not a function of its size, right? so the alignment of the header will just be the ptr alignment, basically.

@thestinger
Copy link
Contributor

@nikomatsakis: well I'm assuming that body_size and body_align don't include padding. So ~bool would have body_size and body_align of 1 but there would be padding at the end up to pointer size.

I don't know where/how this is used so that might not matter.

@nikomatsakis
Copy link
Contributor Author

Ah, I think you are saying that the total size of the struct

struct { header h; bool b; }

would be larger than what this function returns, which is true. I noticed
this too, but seeing as I believe this function is used to compute the
amount of memory to ask malloc for etc this wasn't an issue. But it occurs
to me that it...maybe could be, if malloc wound up giving us less than what
the alignment would have required. Good point.

@thestinger
Copy link
Contributor

malloc already has to give an alignment that's valid for any C type, so it's probably a non-issue.

bors added a commit that referenced this pull request Jun 6, 2013
r? @jld or @graydon

The calculation looks right to me, but perhaps one of you two can double check.  You two seem like you are doing the most recent work in this sort of area.
@bors bors closed this Jun 6, 2013
pnkfelix added a commit to pnkfelix/rust that referenced this pull request Nov 22, 2013
I cannot tell whether the original comment was unsure about the
arithmetic calculations, or if it was unsure about the assumptions
being made about the alignment of the current allocation pointer.

The arithmetic calculation looks fine to me, though.  This technique
is documented e.g. in Henry Warren's "Hacker's Delight" (section 3-1).

(I am sure one can find it elsewhere too, its not an obscure
property.)
@nikomatsakis nikomatsakis deleted the fixme-2699 branch March 30, 2016 16:13
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