feat: Handle invalid CGO handles in C bindings #3966
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3966 +/- ##
===========================================
- Coverage 74.21% 74.00% -0.21%
===========================================
Files 476 476
Lines 43531 43675 +144
===========================================
+ Hits 32304 32321 +17
- Misses 9066 9155 +89
- Partials 2161 2199 +38
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
AndrewSisley
left a comment
There was a problem hiding this comment.
LGTM, just a couple of small comments.
question: Was this broken out of a main C binding refactor, or did you discover that this was entirely responsible for the problems found and you chose to go with this instead of a more comprehensive refactor?
cbindings/errors.go
Outdated
| errInvalidLensConfig string = "invalid lens configuration: %v" | ||
| errMarshallingJSON string = "error marshalling JSON: %v" | ||
| errInvalidKeyType string = "invalid key type: %v" | ||
| errInvalidStorePointer string = "invalid store pointer: %v" |
There was a problem hiding this comment.
todo: These should really be following the same error pattern that we use everywhere else in our code base.
There was a problem hiding this comment.
For everyone's context: Andy and I discussed this issue a bit through direct message. This change will happen, but it will happen through a separate PR/issue.
Relevant issue(s)
Resolves #3965
Description
There were many portions of the c binding code base where a pointer (CGO handle) to a node, transaction, or identity CGO handle was passed in, and assumed valid without any error checking. This would result in a panic and crash if the node were actually invalid. While there is always the chance of the user passing in an invalid handle, it needs to be handled gracefully, which is what now happens. The functions will now return the error as part of the
Resultobject, and return the error message to the user to help indicate what happened.Tasks
How has this been tested?
Specify the platform(s) on which this was tested: