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

Fix transfer full conversion of Atom arrays #180

Merged
merged 1 commit into from Jul 19, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Jul 19, 2017

Instead of the original pointer, the pointer at the end of the array was
being freed.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 19, 2017

Member

As no g_free called, this PR IMHO unneeded

Member

EPashkin commented Jul 19, 2017

As no g_free called, this PR IMHO unneeded

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 19, 2017

Member

@EPashkin: The point is to remove the unnecessary mutability. :)

Member

GuillaumeGomez commented Jul 19, 2017

@EPashkin: The point is to remove the unnecessary mutability. :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 19, 2017

Member

As I know mut ptr: *mut ffi::GdkAtom is syntax sugar for internal mutability and converted by compiler to

unsafe fn from_glib_none_num_as_vec(ptr: *mut ffi::GdkAtom, num: usize) -> Vec<Self> {
let mut ptr = ptr;
...
}

and mut ptr even don't appeared in docs.
So I still think that this already was good enough.

Member

EPashkin commented Jul 19, 2017

As I know mut ptr: *mut ffi::GdkAtom is syntax sugar for internal mutability and converted by compiler to

unsafe fn from_glib_none_num_as_vec(ptr: *mut ffi::GdkAtom, num: usize) -> Vec<Self> {
let mut ptr = ptr;
...
}

and mut ptr even don't appeared in docs.
So I still think that this already was good enough.

Fix transfer full conversion of Atom arrays
Instead of the original pointer, the pointer at the end of the array was
being freed.
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 19, 2017

Member

Sorry, forgot "git commit --amend". There actually is a g_free() here.

Member

sdroege commented Jul 19, 2017

Sorry, forgot "git commit --amend". There actually is a g_free() here.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 19, 2017

Member

For the other function change, see gtk-rs/glib#200 (comment)

Let's use the compiler for preventing mistakes instead of trying to be too clever

Member

sdroege commented Jul 19, 2017

For the other function change, see gtk-rs/glib#200 (comment)

Let's use the compiler for preventing mistakes instead of trying to be too clever

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 19, 2017

Member

@EPashkin @GuillaumeGomez This one is good to go too then?

Member

sdroege commented Jul 19, 2017

@EPashkin @GuillaumeGomez This one is good to go too then?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 19, 2017

Member

Absolutely. Thanks!

Member

GuillaumeGomez commented Jul 19, 2017

Absolutely. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit d44039f into gtk-rs:master Jul 19, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment