Skip to content

Conversation

@Dakror
Copy link
Contributor

@Dakror Dakror commented Nov 22, 2022

Binding a struct pointer member and accessing a member of said struct while the bound pointer is null leads to an access violation in the member fetching process.

This PR checks the struct base pointer for null before trying to dereference and go deeper down the address lookup.

@mikke89
Copy link
Owner

mikke89 commented Nov 23, 2022

Thanks for the PR!

We previously made an alternative approach to this, which also introduces a null type and default values: 3fd100e

Your approach is a bit simpler, but perhaps covers most use cases. What do you think about pros and cons for each approach? Do you have some specific use cases in mind?

@mikke89 mikke89 added enhancement New feature or request data binding labels Nov 23, 2022
@Dakror
Copy link
Contributor Author

Dakror commented Nov 24, 2022

Ah i was working off of master and your change is on a feature branch if i see correctly. Your implementation feels more exhaustive and also the ability to do bool operations on the pointer inside expressions is very nice. I would say go for your implementation. Why is it not merged yet? If there is still work to do on your feature, adding this PR before merging could close the underlying issue for the time being.

@mikke89
Copy link
Owner

mikke89 commented Nov 24, 2022

No worries, I wouldn't expect users to keep track of it.

I wasn't really sure if that feature was desirable, or a direction I wanted to go with. So I ended up putting it on hold. Sounds like there is some interest in it :)

It still needs a bit more work, so I'll merge this PR in the meantime. If you think it could be useful, I'll pick up the other one again at some point. Thanks again for all the contributions!

@mikke89 mikke89 merged commit 97619d4 into mikke89:master Nov 24, 2022
@Dakror Dakror deleted the fix-nullptr-struct branch November 25, 2022 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data binding enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants