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

autogenerate different getter code for strings and wide strings #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jethac
Copy link

@jethac jethac commented May 20, 2016

I was having some difficulty marshalling a native wide string property into a managed string when I realised the code CodeGenDom was giving me was wrong.

The patch is somewhat self-explanatory - when it's a string, we want to:

  1. C-style cast the char* or wchar_t* string pointer (not the address of the pointer), to a void pointer, and
  2. Use the appropriate string-length function (strlen / wcslen) instead of sizeof

Seeing as the repository includes the binaries, I thought about providing rebuilt binaries, but it might be easier to merge without them.

@abeckus
Copy link
Contributor

abeckus commented May 20, 2016

Your changes will break the interface for getting object property.
The size must always be in bytes and data is always is void_.
In C# side you should convert void_ data to string using property encoder.

<code langu
/*

  • Gets a property for the specified object and property ID.
  • @param typeId Type GUID of the object
  • @param propId The property UID to get
  • @param instanceId Instance GUID of the object
  • @param data[out] Pointer to data to be set
  • @param size[out] Pointer to size of data, in bytes
    _/
    extern "C" LVEDRENDERINGENGINE_API void _stdcall LvEd_GetObjectProperty(ObjectTypeGUID typeId, ObjectPropertyUID propId, ObjectGUID instanceId, void* data, int* size);

@jethac
Copy link
Author

jethac commented May 21, 2016

Ahh, breaking the interface would be bad. I'm a little concerned that we're talking past each other at the moment though; with the current pre-patch code, when you have an element in your XSD containing:

<LeGe.NativeProperty name="mystring" nativeName="MyString" nativeType="wchar_t*" access="get">

The auto-generated code will be:

void MyClass_MyString_Get(ObjectGUID instanceId, void** data, int* size)
{
    MyClass* instance = reinterpret_cast<MyClass*>(instanceId);
    static wchar_t* localData;
    localData = instance->GetMyString();
    *data = (void*)&localData;
    *size = sizeof(localData);
}

Which doesn't seem right, seeing as:

  • *data will have the location of localData, not the location that localData points to, and
  • *size will evaluate to sizeof(wchar_t*), rather than the number of bytes in the string

Is that what was intended?

@abeckus
Copy link
Contributor

abeckus commented May 23, 2016

Thanks for clarifying it,
I will test the CodeGen with getter of type wchar_t* and fix the issue in the next update.
please make a local fix, until I update master branch.

Alan

@jethac
Copy link
Author

jethac commented May 24, 2016

Thanks Alan! 😄

@jethac jethac force-pushed the autogen-string-getters branch from ec8b0d0 to 6f47d89 Compare December 12, 2016 05:19
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