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

possible misuse of unsafe.Pointer in gasyncresult.go #74

Closed
sirzooro opened this issue Apr 10, 2024 · 5 comments · Fixed by go-gst/go-glib#7 or #75
Closed

possible misuse of unsafe.Pointer in gasyncresult.go #74

sirzooro opened this issue Apr 10, 2024 · 5 comments · Fixed by go-gst/go-glib#7 or #75

Comments

@sirzooro
Copy link

I tried to add these bindings to my app, and during compilation I got following error:

compilepkg: nogo: errors found by nogo during build-time code analysis:
/.../vendor/github.com/go-gst/go-glib/glib/__main__/vendor/github.com/go-gst/go-glib/glib/gasyncresult.go:86:55: possible misuse of unsafe.Pointer (unsafeptr)

I was able to silence this by adding // nolint:unsafeptr comment to this line. Please fix this or add this comment if this is a false positive.

@RSWilli
Copy link
Member

RSWilli commented Apr 10, 2024

This is the line in question: https://github.com/go-gst/go-glib/blob/main/glib/gasyncresult.go#L86

@RSWilli
Copy link
Member

RSWilli commented Apr 10, 2024

The function in question is the following:

// IsTagged is a wrapper around g_async_result_is_tagged
func (v *AsyncResult) IsTagged(sourceTag uintptr) bool {
	c := C.g_async_result_is_tagged(v.native(), C.gpointer(sourceTag))
	return gobool(c)
}

The linter is complaining about a cast from the uintptr argument to a unsafe.Pointer via C.gpointer. go vet also catches this.

Related to #62

@RSWilli
Copy link
Member

RSWilli commented Apr 10, 2024

@sirzooro are you actively using the IsTagged function that gets reported? The only way to fix this is by changing the argument type, which will be a breaking change.

But the linters are correct, normally how it is now is already broken, these types of casts can lead to really fun to debug segmentation faults. That is why this wont be a major release in terms of semver

@sirzooro
Copy link
Author

No, feel free to change it.

@RSWilli
Copy link
Member

RSWilli commented Apr 10, 2024

FYI this issue was autoclosed and is only actually completed with the merger of #75

@RSWilli RSWilli reopened this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants