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

Clear out with/close/try/finally #20128

Closed
wants to merge 1 commit into from
Closed

Clear out with/close/try/finally #20128

wants to merge 1 commit into from

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Jan 19, 2017

Replaced some close calls with explicit free on pointers.

I did a little bit of whitespace prettying as well.

@kshyatt kshyatt added the libgit2 The libgit2 library or the LibGit2 stdlib module label Jan 19, 2017
@kshyatt kshyatt requested a review from simonbyrne January 19, 2017 17:27
@@ -15,15 +15,13 @@ function Signature(name::AbstractString, email::AbstractString)
@check ccall((:git_signature_now, :libgit2), Cint,
(Ptr{Ptr{SignatureStruct}}, Cstring, Cstring), sig_ptr_ptr, name, email)
sig = GitSignature(sig_ptr_ptr[])
s = Signature(sig.ptr)
close(sig)
s = Signature(sig.ptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

return Signature(sig.ptr) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@simonbyrne
Copy link
Contributor

We probably should wait until we've fixed the segfaults (#20109/#20124).

@kshyatt
Copy link
Contributor Author

kshyatt commented Jan 19, 2017

Fine with me. I'm happy to continue to do style cleanup on this PR while we wait.

if isa(err, GitError)
throw(GitError(Error.Object, err.Code, "cannot create branch `$branch_name` with `$commit_id`"))
else
rethrow(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Derp, thanks <3

@simonbyrne
Copy link
Contributor

Looks like some files under pkg still use with: these need to be removed as well.

@kshyatt kshyatt force-pushed the ksh/nowithnoclose branch 2 times, most recently from 9ee6603 to de93d3b Compare January 22, 2017 22:26
@kshyatt
Copy link
Contributor Author

kshyatt commented Jan 23, 2017

Is the arrayops thing to do with me?

@kshyatt kshyatt force-pushed the ksh/nowithnoclose branch 2 times, most recently from 15c83b8 to a61e579 Compare January 23, 2017 23:57
@kshyatt
Copy link
Contributor Author

kshyatt commented Jan 24, 2017

@Keno @tkelman is the Windows EPERM fail here my fault?

@tkelman
Copy link
Contributor

tkelman commented Jan 24, 2017

Likely. That happens if you try to delete a file that you still have open handles to.

@simonbyrne
Copy link
Contributor

It seems to be failing on Pkg.rm (though different places in each Windows build).

Replaced some `close` calls with explicit `free` on pointers.
@kshyatt
Copy link
Contributor Author

kshyatt commented Apr 7, 2017

This is hella old and conflicted so I'm going to close (ironic, I know) and try again.

@kshyatt kshyatt closed this Apr 7, 2017
@kshyatt kshyatt deleted the ksh/nowithnoclose branch April 7, 2017 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants