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

Add support for implicit marshalling of the eq types #644

Merged
merged 5 commits into from
Sep 14, 2024

Conversation

kant2002
Copy link
Collaborator

@kant2002 kant2002 commented Sep 6, 2024

Also MSVC does not have support for strndup which is C23

Also MSVC does not have support for strndup which is C23
```
const char arr[] = { 'A','B','C','D' };
```
@ForNeVeR ForNeVeR self-requested a review September 6, 2024 11:35
@ForNeVeR ForNeVeR self-assigned this Sep 6, 2024
Comment on lines 207 to 209
var dest = new UTF8String((byte*)StdLibFunctions.Malloc(count));
src.CopyTo(dest, count);
return dest;
Copy link
Owner

Choose a reason for hiding this comment

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

I think this misses the null terminator. Please note this wording from the standard:

If the array pointed to by s does not contain a null within the first size
characters, a null is appended to the copy of the array.

Meaning the result should always be null-terminated, regardless of src[count] being a null byte or not.

@ForNeVeR
Copy link
Owner

I have to say: I am quite confused about all this RESOLUTION_CESIUM business, but let's make this a topic of a new issue, after we finish with this PR.

@kant2002
Copy link
Collaborator Author

Re: RESOLUTION_CESIUM. I almost manage to make it work, except it fails during runtime, since Cesium.Runtime cannot be resolved. I did not pursue investigation, current way of things was present originally. Need time to investigate and fix properly, maybe something simple.

@ForNeVeR
Copy link
Owner

Thanks!

@ForNeVeR ForNeVeR merged commit 7735ae6 into ForNeVeR:main Sep 14, 2024
3 checks passed
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.

2 participants