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

Fixes an issue related to a wrong variable type (#182) #194

Merged
merged 1 commit into from
Aug 8, 2012
Merged

Fixes an issue related to a wrong variable type (#182) #194

merged 1 commit into from
Aug 8, 2012

Conversation

lpinca
Copy link
Contributor

@lpinca lpinca commented Aug 8, 2012

The issue was generated by passing the negative value of an unsigned variable as parameter of the function AdjustAmountOfExternalAllocatedMemory.

The version of V8 included in node v0.8.x changed the param type of the function AdjustAmountOfExternalAllocatedMemory from int to intptr_t.

The result of this change is that if foo is an unsigned variable, passing -foo to AdjustAmountOfExternalAllocatedMemory is the same of passing the value (UINT_MAX - foo + 1) and not the expected - foo.

Changing the type of the passed variable to int16_t fixed the issue.

tj added a commit that referenced this pull request Aug 8, 2012
Fixes an issue related to a wrong variable type (#182)
@tj tj merged commit 1d0e86a into Automattic:master Aug 8, 2012
@tj
Copy link
Contributor

tj commented Aug 9, 2012

I suppose we could just cast no?

@lpinca
Copy link
Contributor Author

lpinca commented Aug 9, 2012

Ok, but it doesn't have much sense to declare a variable unsigned when you plan on getting its negative value.
Why don't just declare it intptr_t at this point?

@tj
Copy link
Contributor

tj commented Aug 9, 2012

well it doesn't really make sense to give the length of something a signed value either, but keep in mind we have a range of 4294967296 with the unsigned int so we should be safe :p node does this all over too with size_t

@Dman9296
Copy link

Dman9296 commented Aug 1, 2020

Nothing but the best

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.

None yet

4 participants