views:

607

answers:

3

I'm usign C# with P/Invoke to acces to a dll method. The definition of the method is the following:

[DllImport("userManager.dll")]
static extern int GetUsers(out IntPtr userList);

And the struct layout I've done is the following:

    [StructLayout(LayoutKind.Sequential)]
    public class USER_LIST
    {
        public uint NumUsers;
        [MarshalAs(UnmanagedType.ByValArray)]
        public USER_LIST_ITEM [] List;
    }

    [StructLayout(LayoutKind.Sequential)]
    public class USER_LIST_ITEM
    {
        [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 260)]
        public string name;
        [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 256)]
        public string address;
    };

Original structs:

typedef struct user_list {
   unsigned short NumUsers;
   USER_LIST_ITEM List[VARLEN];
} USER_LIST

typedef struct user_list_item {
   char name[260];
   unsigned char address[256];
} USER_LIST_ITEM

But I get an error when I try to unmarshall it:

        USER_LIST userList = new USER_LIST();
        // Prepare pointer 
        IntPtr uList = Marshal.AllocHGlobal(Marshal.SizeOf(userList));
        Marshal.StructureToPtr(userList, uList, false);
        result = GetUsers(out uList);

        Marshal.PtrToStructure(uList, userList); <--

The runtime has encountered a fatal error. The address of the error was at 0x79f82af6, on thread 0x464. The error code is 0xc0000005. This error may be a bug in the CLR or in the unsafe or non-verifiable portions of user code. Common sources of this bug include user marshaling errors for COM-interop or PInvoke, which may corrupt the stack.

I get the NumUsers property right, but it seems the error occurs when unmarshalling the array. Any thoughts?

+3  A: 

That is because List has not been allocated yet.

You will need initialize all the fields.

Another problem I see is with the following:

IntPtr uList = Marshal.AllocHGlobal(Marshal.SizeOf(userList));
...
result = GetUsers(out uList);

Are you sure that out should not be ref? Else there is no point (not sure if ref is correct either).

Update: Looking at your code again, you should be doing this (and avoid that memory leak poking your eye).

IntPtr uList;
var result = GetUsers(out uList);

var userlist = (USER_LIST) Marshal.PtrToStructure(ulist, typeof(USER_LIST));

Marshal.FreeHGlobal(ulist); // pray here or shoot the author of the C function

Update again:

Your p/invoke signature is likely wrong or you are interpreting it wrong.

I can guess it probably something like:

int GetUsers(USER_LIST* ulist);

And that what you have is not the same thing.

If this is case, the solution is easy.

Change USER_LIST to a class (but keep sequential layout) and use

// pinvoke sig
int GetUsers(USER_LIST ulist);

var ulist = new USER_LIST();
// initialize fields
var r = GetUsers(ulist);

-- or --

Call it by ref.

// pinvoke sig
int GetUsers(ref USER_LIST ulist);

var ulist = new USER_LIST();
// initialize fields
var r = GetUsers(ref ulist);

This way, you dont have to mess with manual marshalling, and I cant see anymore potential for memory leaks.

Final update:

Given the signature you posted, it looks like GetUsers returns a pointer to a list of USER_LIST with the return value being the count. Nice memory leak there.

Anyways, I would probably experiment with an unsafe approach here, and just walk thru the result , and make sure everything gets freed. (I still think you should shoot the author).

leppie
The return is either 0 or 1, just to show the success of the call
Sergi
@Sergi: then you have a big leak and a nice buffer overrun to boot!
leppie
@Sergi: going unsafe will probably look just like the C, and might be the easiest.
leppie
+1  A: 

I think your original code isn't probably so wrong. You've probably just used the wrong overload of Marshal.PtrToStructure.

Have you tried this?

[DllImport("userManager.dll")]
static extern int GetUsers(out IntPtr userList);

[DllImport("userManager.dll")]
static extern void UMFree(IntPtr userList);

static void Main()
{
    IntPtr userList;               // no need to allocate memory in managed code;
    GetUsers(out userList);        // memory is allocated by native function
    USER_LIST u = (USER_LIST)Marshal.PtrToStructure(userList, typeof(USER_LIST));
    UMFree(userList);
}


Using unsafe code:

public unsafe struct USER_LIST
{
    public uint numUsers;
    public USER_LIST_ITEM* list;
}

public unsafe struct USER_LIST_ITEM
{
    public fixed byte name[260];
    public fixed byte address[256];
}

class Program
{
    [DllImport("userManager.dll")]
    static unsafe extern int GetUsers(USER_LIST** userList);

    [DllImport("userManager.dll")]
    static unsafe extern int UMFree(USER_LIST* userList);

    private static unsafe void Main()
    {
        USER_LIST* list;
        GetUsers(&list);
        UMFree(list);
    }
}
dtb
Thanks for the snippet, but it's not working, I get the same error when PtrToStructure, I don't know if the structure definition is right, 'cause the code seem all right to me as well
Sergi
I think the problem is the variable-sized array in the USER_LIST. How can the marshaller know it's size (it doesn't know that the size is stored in `NumUsers`). Using unsafe code is probably the best solution.
dtb
BTW, your structs in C# are actually declared as classes. Have you tried changing them to `struct`?
dtb
I've added an example using unsafe code.
dtb
+3  A: 

If you specify an array in a structure used as an out parameter, you need to tell the marshaler what length is the array going to be. With your code, the marshaler is probably allocating a zero-length array or just using null, which produces the crash. Unfortunately there seems to be no way to specify a variable-length out array as a member of a structure, because MarshalAs.SizeParamIndex only works for methods. You might get away with specifying a large, constant-size array using MarshalAs.SizeConst, but generally you'd have to parse the (presumably callee-allocated) return buffer like this:

var count = Marshal.ReadInt32 (uList) ;
var users = new List<USER_LIST_ITEM>  () ;
var ptr   = (long)uList + 4 ;
for (int i = 0 ; i < count ; ++i)
{
    users.Add (Marshal.PtrToStructure (typeof (USER_LIST_ITEM), 
        new IntPtr (ptr))) ;
    ptr += Marshal.SizeOf (typeof (USER_LIST_ITEM)) ;
}

You'll have to pay extra attention to alignment&padding and 32/64 bit issues.

Anton Tykhyy
Doing the manual mashalling worked fine. Thx
Sergi