Question

I'm calling the IDiaSourceFile::get_checksum member function, which has the following auto-generated .NET signature:

void get_checksum(uint cbData, out uint pcbData, byte[] pbData);

Note that the parameter pbData is an out parameter. Do I need to worry about pinning the array passed in here when someone calls get_checksum?

(Background:

I inherited some code that looks like this:

unsafe
{
    fixed (byte* p = scratch_hash)
    {
        sourceFile.get_checksum(c, out c, scratch_hash);
    }
}

which pins scratch_hash as p and then never actually uses p. I've not seen anything like this before and given the state of the surrounding code I suspect the pin is completely unnecessary here

)

Was it helpful?

Solution

With the current definition:

void get_checksum(uint cbData, out uint pcbData, byte[] pbData);

You don't need to pin pbData. However, you'd need to pre-allocate an array for the returned the data, but you don't know the size in advance. The method will fail if the array size you pass via cbData is not large enough, and your current method signature doesn't allow to find out the buffer size.

The original C++ declaration does allow that:

HRESULT get_checksum ( 
   DWORD  cbData,
   DWORD* pcbData,
   BYTE   data[]
);

data [in, out] A buffer that is filled with the checksum bytes. If this parameter is NULL, then pcbData returns the number of bytes required.

So, a more efficient approach might be to declare and use it like this:

void get_checksum(
    uint cbData, 
    out uint pcbData, 
    IntPtr data);

// get size
uint size;
obj.get_checksum(0, out size, IntPtr.Zero);

// get data
var buff = new byte[size];
unsafe
{
    fixed (byte* p = buff)
    {
        uint cbData;
        obj.get_checksum(size, out cbData, (IntPtr)p);
        if (size != cbData)
            throw new InvalidOperationException("cbData");
    }
}

If you don't want (or can't) use unsafe code, here's an alternative:

// get size
uint size;
obj.get_checksum(0, out size, IntPtr.Zero);

// get the data
byte[] buff;
var p = Marshal.AllocHGlobal((int)size);
try
{
    uint cbData;
    obj.get_checksum(size, out cbData, p);
    if (size < cbData)
        throw new InvalidOperationException("cbData");
    buff = new byte[cbData];
    Marshal.Copy(p, buff, 0, (int)cbData);
}
finally
{
    Marshal.FreeHGlobal(p);
}

OTHER TIPS

No, you don't have to do it in this case - and you can't, even.

However, you might want to change that signature if you run into performance issues, because the marshaller does a lot of work to pass the byte array to the native method - it has to allocate new memory, copy the managed byte array to that, and then deallocate the memory again.

This means that the code you posted is complete nonsense, because while it does fix the scratch_hash array, it doesn't use that fixed pointer anyway.

If you're only calling the method once in a while and the byte array is relatively small, this can be safely ignored. However, if you do find that this causes unnecessary strain on your application, fixing could help - but you'd have to change the signature:

unsafe void get_checksum(uint cbData, out uint pcbData, byte *pbData);

Then you can use this:

fixed (byte *p = &scratch_hash[0])
  sourceFile.get_checksum(c, out c, p);

All this said, note that fixing a managed object might cause issues of its own (for example, it pretty much kills any chance of heap compaction if your object is on top of the heap). Fixing should be done sparingly, and only for extremely short times - the only other reasonable alternative is to allocate the objects early, and keeping them together (for example, when using asynchronous I/O, you need a fixed reference, usually for as long as the application runs - so just allocate the buffers as early as possible, and you'll be mostly fine).

Also, I'm still finding that signature a bit wild. Could you share the header file's definition of the signature?

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top