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

CGOError --> CGOErrCode #12499

Merged
merged 8 commits into from
May 9, 2022
Merged

CGOError --> CGOErrCode #12499

merged 8 commits into from
May 9, 2022

Conversation

ibeckermayer
Copy link
Contributor

Rather than passing errors via C-strings, we pass them as integers which removes a major potential memory leak/error footgun

(Credit to @zmb3 for this idea)

…ich removes a major potential memory leak/error footgun
@ibeckermayer ibeckermayer requested review from LKozlowski and zmb3 May 6, 2022 20:20
Copy link
Contributor

@strideynet strideynet left a comment

Choose a reason for hiding this comment

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

I'm not overly familiar with Rust, but the Go/CGo changes look sensible.

Am I right in thinking that with this change we lose some detail on the Go side as to what error actually occurred, but that these errors are now logged in the Rust side ?

@zmb3
Copy link
Collaborator

zmb3 commented May 6, 2022

Am I right in thinking that with this change we lose some detail on the Go side as to what error actually occurred, but that these errors are now logged in the Rust side ?

The motivation for this was actually the opposite. We were passing a string from Go to Rust, just so Rust could log the error and then terminate. Now, we log the error on the Go side, and then just indicate to Rust that there was a failure. It's a lot easier to pass an integer than a string across the FFI.

(Though it looks like this was applied in both directions, so you're correct that Rust is also passing only an error code to Go).

@ibeckermayer ibeckermayer requested a review from zmb3 May 7, 2022 00:17
@ibeckermayer ibeckermayer merged commit b7b7ac2 into master May 9, 2022
ibeckermayer pushed a commit that referenced this pull request Jun 16, 2022
ibeckermayer pushed a commit that referenced this pull request Jun 21, 2022
ibeckermayer pushed a commit that referenced this pull request Jun 29, 2022
* Abstract-out chunked read logic (#11616)

* reuse `vchan::Client` to add the general header and break messages into chunks (#11714)

* `CGOError` --> `CGOErrCode` (#12499)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants