Skip to content

Add RawContentStream.#324

Merged
nulltoken merged 1 commit intolibgit2:vNextfrom
txdv:rawcontentstream
Feb 10, 2013
Merged

Add RawContentStream.#324
nulltoken merged 1 commit intolibgit2:vNextfrom
txdv:rawcontentstream

Conversation

@txdv
Copy link
Contributor

@txdv txdv commented Feb 10, 2013

Holds the rawcontent objectsafe wrapper.

Some context: eb304fb#commitcomment-2459256

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather keep the Proxy method and its current signature. This would allow Blob.cs to stay unmodified.

Regarding the Proxy implementation, could you please implement it so a pointer to the NativeMethods.git_blob_rawcontent method is passed to the RawContentStream ctor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I need the ObjectSafeWrapepr and the pointer in the class. How do I retrieve it with the current signature?

@txdv
Copy link
Contributor Author

txdv commented Feb 10, 2013

Keeping the Proxy method and its current signature is impossibru, or it would force me to create the ObjectSafeWrapper two times, which is not a good idea.

@nulltoken
Copy link
Member

Keeping the Proxy method and its current signature is impossibru, or it would force me to create the ObjectSafeWrapper two times, which is not a good idea.

How about this?

public static UnmanagedMemoryStream git_blob_rawcontent_stream(RepositorySafeHandle repo, ObjectId id, Int64 size)
{
     return new RawContentStream(id, repo, NativeMethods.git_blob_rawcontent, size);
}

@txdv
Copy link
Contributor Author

txdv commented Feb 10, 2013

Still the problem persists, you want me to create 2 ObjectSafeWrappers if the current signature is to stays.
Look at the the code inside, how it gets the pointer:

using (var obj = new ObjectSafeWrapper(id, repo))
{
    IntPtr ptr = NativeMethods.git_blob_rawcontent(obj.ObjectPtr);

@nulltoken
Copy link
Member

Still the problem persists, you want me to create 2 ObjectSafeWrappers

I'm sorry. I don't follow you. You already create an ObjectSafeWrapper in the RawContentStream``. Why don't you use it?

Provided the Proxy implementation is

public static UnmanagedMemoryStream git_blob_rawcontent_stream(RepositorySafeHandle repo, ObjectId id, Int64 size)
{
     return new RawContentStream(id, repo, NativeMethods.git_blob_rawcontent, size);
}

The RawContentStream could be as follows, with only one ObjectSafeWrapper

using System;
using System.IO;
using LibGit2Sharp.Core;
using LibGit2Sharp.Core.Handles;

namespace LibGit2Sharp
{
    internal class RawContentStream : UnmanagedMemoryStream
    {
        private readonly ObjectSafeWrapper wrapper;

        public RawContentStream(ObjectId id, RepositorySafeHandle repo,
            Func<GitObjectSafeHandle, IntPtr> bytePtrProvider, long length) :
            this(new ObjectSafeWrapper(id, repo), bytePtrProvider,length)
        {
        }

        unsafe RawContentStream(ObjectSafeWrapper wrapper,
            Func<GitObjectSafeHandle, IntPtr> bytePrv, long length)
            : base((byte *)(bytePrv(wrapper.ObjectPtr)).ToPointer(), length)
        {
            this.wrapper = wrapper;
        }

        protected override void Dispose(bool disposing)
        {
            base.Dispose(disposing);
            wrapper.SafeDispose();
        }
    }
}

Or am I missing something?

@txdv
Copy link
Contributor Author

txdv commented Feb 10, 2013

Ok I understood now

@txdv
Copy link
Contributor Author

txdv commented Feb 10, 2013

No, still the same, please show me the content of your proposed git_blob_rawcontent_stream. The signature has to be changed to take the ObjectSafeWrapper at least as an argument, otherwise the creation of 2 ObjectSafeWrappers will still exist.

@nulltoken
Copy link
Member

No, still the same, please show me the content of your proposed git_blob_rawcontent_stream.

See the comment above 😉

Holds the rawcontent objectsafe wrapper.
@txdv
Copy link
Contributor Author

txdv commented Feb 10, 2013

These indirections confused me.

As an additional note, the contract here dictates that the stream can be accessed and disposed only in the using (repo) block, otherwise it might return nasty exceptions and bring the entire application down.
However, there is a way to throw just a simple exception using GCHandle if the Repository has been already closed.
I'll add the commit here to show of what I am thinking about.

@nulltoken nulltoken merged commit 2f18086 into libgit2:vNext Feb 10, 2013
@nulltoken
Copy link
Member

Very neat addition. Thanks!

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