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

add FileChooser.SetUri and fix a memory leak of FileChooser.GetFilename #370

Merged
merged 7 commits into from Feb 16, 2018

Conversation

lidaobing
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Feb 15, 2018

Codecov Report

Merging #370 into master will increase coverage by 1.22%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #370      +/-   ##
=========================================
+ Coverage    3.75%   4.98%   +1.22%     
=========================================
  Files           1       1              
  Lines        4447    4477      +30     
=========================================
+ Hits          167     223      +56     
+ Misses       4275    4245      -30     
- Partials        5       9       +4
Impacted Files Coverage Δ
gtk/gtk.go 4.98% <100%> (+1.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b67e0d...6d80ecd. Read the comment docs.

gtk/gtk.go.h Outdated
for(GSList* l = gs; l != NULL; l = l->next) {
GSList* l;

for(l = gs; l != NULL; l = l->next) {
Copy link
Owner

@mattn mattn Feb 16, 2018

Choose a reason for hiding this comment

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

please omit () Ah, this is C code. Sorry

@lidaobing lidaobing changed the title add FileChooser.SetUri and fix a memory leak of FileChooser.GetFilename [DO NOT MERGE] add FileChooser.SetUri and fix a memory leak of FileChooser.GetFilename Feb 16, 2018
@lidaobing
Copy link
Contributor Author

lidaobing commented Feb 16, 2018

@mattn I have run testcase about 20 times on my machine, and unfortunately the result is not consistent, sometimes the GetFilename() does not return as expected. sometimes it triggered a critical GTK warning in UnselectUri[1], I need some deep inspect in it. please do not merge, thanks.

[1]

(<unknown>:76378): Gtk-CRITICAL **: gtk_tree_selection_unselect_iter: assertion 'selection->tree_view->priv->model != NULL' failed
SIGTRAP: trace trap
PC=0x4f5cf07 m=0 sigcode=1
signal arrived during cgo execution

goroutine 9 [syscall, locked to thread]:
runtime.cgocall(0x4319720, 0xc42003fd00, 0xc42003fd28)
	/usr/local/Cellar/go/1.9.1/libexec/src/runtime/cgocall.go:132 +0xe4 fp=0xc42003fcd0 sp=0xc42003fc90 pc=0x400a204
github.com/mattn/go-gtk/gtk._Cfunc_gtk_file_chooser_unselect_uri(0x604d470, 0x560e380)
	github.com/mattn/go-gtk/gtk/_test/_obj_test/_cgo_gotypes.go:9200 +0x41 fp=0xc42003fd00 sp=0xc42003fcd0 pc=0x42c8531
github.com/mattn/go-gtk/gtk.(*FileChooser).UnselectUri.func1(0x604d470, 0x560e380)
	/Users/lidaobing/go/src/github.com/mattn/go-gtk/gtk/gtk.go:8162 +0x6a fp=0xc42003fd38 sp=0xc42003fd00 pc=0x42f749a
github.com/mattn/go-gtk/gtk.(*FileChooser).UnselectUri(0xc420010e28, 0xc42003fdc8, 0x1b)
	/Users/lidaobing/go/src/github.com/mattn/go-gtk/gtk/gtk.go:8162 +0x7a fp=0xc42003fd68 sp=0xc42003fd38 pc=0x42e2a9a
github.com/mattn/go-gtk/gtk_test.TestFILE_CHOOSER(0xc4200fc4b0)
	/Users/lidaobing/go/src/github.com/mattn/go-gtk/gtk/gtk_test.go:63 +0xa75 fp=0xc42003ffa8 sp=0xc42003fd68 pc=0x4312835
testing.tRunner(0xc4200fc4b0, 0x440ada8)
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:746 +0xd0 fp=0xc42003ffd0 sp=0xc42003ffa8 pc=0x40eb030
runtime.goexit()
	/usr/local/Cellar/go/1.9.1/libexec/src/runtime/asm_amd64.s:2337 +0x1 fp=0xc42003ffd8 sp=0xc42003ffd0 pc=0x4063b21
created by testing.(*T).Run
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:789 +0x2de

@lidaobing lidaobing changed the title [DO NOT MERGE] add FileChooser.SetUri and fix a memory leak of FileChooser.GetFilename add FileChooser.SetUri and fix a memory leak of FileChooser.GetFilename Feb 16, 2018
@lidaobing
Copy link
Contributor Author

@mattn I remove the check in test, I think now you can merge.

@mattn
Copy link
Owner

mattn commented Feb 16, 2018

okay, merging. thanks

@mattn mattn merged commit 5a311a1 into mattn:master Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants