4

Below I have a code snippet from c++. I need to return array of pointers (to TempStruct).

The problem is that on c# side I get only one element. On the other I get AV.

**C++**
extern "C" __declspec(dllexport) void GetResult(TempStruct** outPtr, long *size)
{       
    *outPtr = (TempStruct*)new TempStruct*[2];

     outPtr[0] = new TempStruct("sdf", 123);
     outPtr[1] = new TempStruct("abc", 456);

    *size = 2;      
}

**C#**    
[DllImport("test.dll", CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)]
public static extern void GetResult(out IntPtr outPtr, out int size);

IntPtr ptr = IntPtr.Zero;
int length;        
GetResult(out ptr, out length);

int size = Marshal.SizeOf(typeof(TempStruct));
TempStruct[] someData2 = new TempStruct[length];

for (int i = 0; i < length; i++)
{
   IntPtr wskptr = (IntPtr)(ptr.ToInt64() + (size * i));
   someData2[i] = (TempStruct)Marshal.PtrToStructure(wskptr, typeof(TempStruct));            
} 
John
  • 1,834
  • 5
  • 32
  • 60
  • 1
    Don't you mean e.g. `(*outPtr)[0]`? And shouldn't the `outPtr` argument be a *tripple* pointer (i.e. `TempStruct*** outPtr`) since you allocate an array of pointers? The casting you do when allocating memory is masking that last error, don't ever cast the result of `new`. – Some programmer dude Jun 24 '15 at 11:18

2 Answers2

4

You are doing it wrong.

You are mixing pointer types.

By using the new TempStruct() you are creating an array of pointers to TempStruct. The example I gave you created an array of TempStruct. See the difference?

Now... TempStruct** outPtr should be TempStruct*** outPtr (because you want to return (*) an array (*) of pointers (*)... Or TempStruct**& if you prefer :-)

Change this line

someData2[i] = (TempStruct)Marshal.PtrToStructure(Marshal.ReadIntPtr(wskptr), typeof(TempStruct));

Because you must read the single pointers.

I do hope you are deleting the various TempStruct with delete and using the

delete[] ptr;

operator to delete the array of structures.

Full example:

C++:

struct TempStruct
{
    char* str;
    int num;

    // Note the strdup. You don't know the source of str.
    // For example if the source is "Foo", then you can't free it.
    // Using strdup solves this problem.
    TempStruct(const char *str, int num) 
        : str(strdup(str)), num(num)
    {
    }

    ~TempStruct()
    {
        free(str);
    }
};

extern "C"
{
    __declspec(dllexport) void GetResult(TempStruct ***outPtr, int *size)
    {
        *outPtr = new TempStruct*[2];

        (*outPtr)[0] = new TempStruct("sdf", 123);
        (*outPtr)[1] = new TempStruct("abc", 456);

        *size = 2;
    }

    __declspec(dllexport) void FreeSomeData(TempStruct **ptr, int size)
    {
        for (int i = 0; i < size; i++)
        {
            delete ptr[i];
        }

        delete[] ptr;
    }
}

C#:

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi, Pack = 1), Serializable]
internal struct TempStruct
{
    public string str;
    public int num;
}

[DllImport("NativeLibrary.dll", CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)]
static extern void GetResult(out IntPtr outPtr, out int numPtr);

[DllImport("NativeLibrary.dll", CallingConvention = CallingConvention.Cdecl)]
static extern void FreeSomeData(IntPtr ptr, int num);

// C++ will return its TempStruct array in ptr
IntPtr ptr;
int size;

GetResult(out ptr, out size);

TempStruct[] someData2 = new TempStruct[size];

for (int i = 0; i < size; i++)
{
    IntPtr ptr2 = Marshal.ReadIntPtr(ptr, i * IntPtr.Size);
    someData2[i] = (TempStruct)Marshal.PtrToStructure(ptr2, typeof(TempStruct));
}

// Important! We free the TempStruct allocated by C++. We let the
// C++ do it, because it knows how to do it.
FreeSomeData(ptr, size);

Note that you don't need [Serializable] and Pack=1 on the C# struct

More correct for the C++:

__declspec(dllexport) void GetResult(TempStruct **&outPtr, int &size)
{
    outPtr = new TempStruct*[2];

    outPtr[0] = new TempStruct("sdf", 123);
    outPtr[1] = new TempStruct("abc", 456);

    size = 2;
}

It is more correct because both outPtr and size can't be NULL. See https://stackoverflow.com/a/620634/613130 . The C# signature is the same.

Community
  • 1
  • 1
xanatos
  • 109,618
  • 12
  • 197
  • 280
  • Still I can only read only 1st item. I changed parameter to triple pointers, and changed c# code as you wrote above – John Jun 24 '15 at 11:28
  • `delete[] ptr` isn't enough. Code in the question requires each item to be deleted too. Which is why I believe that the C++ code is wrong. – David Heffernan Jun 24 '15 at 11:31
  • @DavidHeffernan Yep... Corrected. – xanatos Jun 24 '15 at 11:36
  • @John See full example. – xanatos Jun 24 '15 at 11:36
  • @xanatos In c++: "*outPtr = new TempStruct*[2];" will not I have AV, as outPtr is not yet initialized and I am using the * - pointer here? – John Jun 24 '15 at 13:31
  • @John That example works as is. Your question is equivalent to asking: `*size = 2;` will have an AV? Can you do `*size = 2;`? If yes, then you can do `*outPtr = new something` – xanatos Jun 24 '15 at 13:34
  • @xanatos Maybe that was a stupid question, but it was as a result of AV here: TempStruct ***saTemp = NULL; *saTemp = new TempStruct*[*size]; – John Jun 24 '15 at 14:13
  • 1
    @John The `outPtr` in the example isn't normally `NULL`. If you used that method from C++, you would: `TempStruct **ptr; int size; GetResult(&ptr, &size)`, so you would pass a value for `outPtr`: `&ptr`. The same in C#, you use `out ptr`. Clearly you could add a `NULL` check, because someone could do a `GetResult(NULL, NULL)`. Technically the "right" way to write it in C++ would be `void GetResult(TempStruct **&outPtr, int &size)`, because both `outPtr` and `size` can't be `NULL` (see http://stackoverflow.com/a/620634/613130) – xanatos Jun 24 '15 at 14:15
3

The C++ code is wrong. It's returning an array of pointers to struct. The fact that you cast the value returned by new should have alerted you to the fact that you made a mistake. You want to return an array of struct.

It should be:

*outPtr = new TempStruct[2];
(*outPtr)[0].str = "sdf";
(*outPtr)[0].i = 123;
(*outPtr)[1].str = "abc";
(*outPtr)[1].i = 456;
*size = 2;      
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • Don't *you* mean e.g. `(*outPtr)[0]`? :) – Some programmer dude Jun 24 '15 at 11:26
  • @JoachimPileborg Yes. Thank you. I'm just rubbish at C++! – David Heffernan Jun 24 '15 at 11:30
  • @DavidHeffernan The only problem is that you can't easily use parameterized constructors with that syntax. Someone had asked that [here](http://stackoverflow.com/q/4754763/613130) and the solutions where horrible. It is possible to use [this one](http://stackoverflow.com/a/4756306/613130), but I would prefer to go to the dentist... :-) – xanatos Jun 24 '15 at 11:48
  • @xanatos Well yes, but I'd prefer that to all that extra allocation and indirection. And if the caller can know how many elements are needed then the whole thing can be done by the p/invoke marshaller with no explicit allocation. In the C++ code I'd just make a function that returns a new value: `TempStruct MakeNewStruct(const char* str, const int i)` – David Heffernan Jun 24 '15 at 11:52