Skip to content

C++: Add cases in the Allocation model.#4824

Merged
jbj merged 4 commits intogithub:mainfrom
geoffw0:modelchanges5
Dec 18, 2020
Merged

C++: Add cases in the Allocation model.#4824
jbj merged 4 commits intogithub:mainfrom
geoffw0:modelchanges5

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Dec 14, 2020

While working on the #4783 I noticed a few holes in our allocation models. I've also added test cases for strdup, a standard library function which it turns out we already support.

@geoffw0 geoffw0 added the C++ label Dec 14, 2020
@geoffw0 geoffw0 requested a review from a team as a code owner December 14, 2020 14:05
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

Looks good! I only have one comment (and then we need an internal PR as well, of course).

… of the SysAlloc stuff is beyond our current models, supporting just SysFreeString as we do is OK.
@geoffw0
Copy link
Contributor Author

geoffw0 commented Dec 16, 2020

There was a failing (internal) test and I had to remove support for SysAllocString. It turns out supporting all of the SysAlloc stuff pushed beyond the flexibility offered by our current models, supporting just SysAllocString and SysFreeString leads to problems where the code uses other functions as well, but supporting just SysFreeString as we previously did is safe (and it is rarely used to free stuff that was allocated in ways we do model, I think).

When we have support for access paths in our models, I think we should be able to do better.

@jbj jbj merged commit fd7dec7 into github:main Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants