-
Notifications
You must be signed in to change notification settings - Fork 206
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
Headers and debug improvements #338
base: main
Are you sure you want to change the base?
Headers and debug improvements #338
Conversation
Added callback arg names for the interface bind
Warning: Changes size of the event struct.
Allow full debugging with MSVC on debug builds: Looks good, thank you 👍 use const char* when const for public headers: Looks good too, but Magic cookie addition to events to avoid invalid calls: If the |
Officially a const char * shouldn't change, and we need to copy something into it after assignment. If there was a _webui_strdup would certainly work. Still I can avoid the extra code and just quietly cast the const char * to a char* for that scoped and we won't tell anyone. We can't just use a char * for window_url in general as we assign content to it, which is a const (although again could cast it). So of the two slightly hacky things i think I prefer the cast into strcpy.
Not quite. A user could hold on to an event until a random time after execution and then attempt to set the result on it despite it being freed. |
Sure, the thing is we should avoid changing |
This has 3 commits can submit them separately or remove any not wanted but some of the lines were too close to avoid conflicts otherwise.
Most of these changes are minor, adding better MSVC debug flags, switching some char* to const char* when they are const. Adding names to the interface_bind function pointer. In addition to making things a bit more clear it assists autogeneration of the code with a tool like CPPSharp. More details on that below.
The final item that is here is adding a magic cookie to event structs. I dislike the idea of expanding the struct to include a memory check but it is one of two ways I thought to do this effectively. Events are only executed in serial one at a time and do not support callbacks. This may make users more likely to use them in an async fashion and potentially try to set the result after they should (or call another function on them). As we try to use the event ID directly this could cause use after free issues (but with something like interfaces may not be invalid due to the event buffer).
I thought about just adding this with a DEBUG compile time flag, but changing the event struct size based on that seemed like a bad idea. The only other option takes advantage of the serial call limitation. We could store the pointer to the current event struct, and the current event's id (for interface callbacks). We could then assert if the pointer/id number that come in don't match.
Auto Code Generation
With these changes no modifications are needed to generate a fairly 1:1 api: https://github.com/mitchcapper/WebUIAutoBuild/blob/master/CodeGen/WebUIGeneratorDirect.cs
resulting in: https://github.com/mitchcapper/WebUIAutoBuild/blob/master/Generated/WebUI.cs
With a more complicated generator can generate it into a more standardized class form: https://github.com/mitchcapper/WebUIAutoBuild/blob/master/CodeGen/WebUIGeneratorClass.cs
I put together a quick sample for what each looks like when used at:
https://github.com/mitchcapper/WebUIAutoBuild/tree/master/ConsoleSampleApp
but for the class version its pretty spot on:
Note these were not to replace the actual c# lib ( https://github.com/Juff-Ma/WebUI.NET ) but I generated for some testing.