Skip to content

Try to get rid of SafeDispose() calls #735

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

Merged
merged 2 commits into from
Jun 2, 2014
Merged

Conversation

nulltoken
Copy link
Member

SafeDispose() extension method takes care of ensuring the handle is valid before disposing it.

Let's try and get rid of this by moving the checks in SafeHandleBase.ReleaseHandle()

@nulltoken
Copy link
Member Author

So, we can't get rid of SafeDispose() as is.

However, we can make it much simpler and makes its goal more obvious: Avoiding to have check if the handle is null before invoking Dispose().

Along the way I've found some places that could be simplified into a plain using () { } code block.

@@ -442,10 +442,8 @@ internal GitObject LookupInternal(ObjectId id, GitObjectType type, FilePath know

GitObjectSafeHandle obj = null;
Copy link
Member

Choose a reason for hiding this comment

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

Move declaration into using?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@nulltoken
Copy link
Member Author

@Therzok One one your previous comment (that I can't seem to find anymore), was hinting at simplifying the code in WorkdirAndIndexToTree().

This is now done and the returned diff handle should be disposed (even on error) by the callers of BuildDiffList().

@Therzok
Copy link
Member

Therzok commented Jun 2, 2014

I deleted it right after. I was sleepy and unsure of what I wrote. :D

nulltoken added 2 commits June 2, 2014 19:36
git_branch_delete() only frees the handle upon success.
@nulltoken nulltoken merged commit 9e82f30 into vNext Jun 2, 2014
@nulltoken nulltoken added this to the v0.18.0 milestone Jun 2, 2014
@nulltoken nulltoken deleted the ntk/disposable_the_sequel branch June 2, 2014 18:02
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.

3 participants