What were they thinking?

Bugs are frustrating, but when you write code for a living you expect them and live with them as a fact of life.  They are a lot harder to swallow when they are introduced by other libraries, especially when those libraries work counter to what you would expect.  It’s even more frustrating when it’s in a library like the Compact Framework itself.


Recently we got a bug report for our ConnectionManager in some code that I know we tested – basically the Description returned by the DestinationInfo class was returning odd data and concatenating to it did bad things.  Experience told me that it sounded like the data coming back from the native API was not getting properly truncated at a NULL.  Not an unusual mistake, so I went to find it.


Description = Marshal2.PtrToStringUni(baseAddr, 16, 256);
int nullPos = Description.IndexOf(
);
if (nullPos > -1) Description = Description.Substring(0, nullPos);


The bad news was the code looked right – we were looking for NULL and trimming at it.  The only way a problem could arise is if the buffer was non-zero at the start.  So off to look at that code.  Here’s where the allocation is made:


hDestInfo = Marshal.AllocHGlobal(DestinationInfo.NativeSize);


Again, it looked right.  However, talking with Alex Feinman he said that the Marshal Alloc functions do no zero memory.  In the earlier versions of this code we used our internal MarshalEx, which P/Invoked LocalAlloc with the LPTR parameter, which zeros everything at allocation.  Why the hell the CF doesn’t do that one can only guess, but it makes no sense in my book.  Who would want to allocate memory and not zero it?  Worse still is that MSDN doesn’t say that the memory is not zeroed.


So then, changing from our MarshalEx function to the CF’s Marshal function introduced an error.  So how do we fix it?  There’s no CF equivalent to a memset (again, this is a WTF in my book, but there are a few language limitations like this that irritate me – try taking an IntPtr and turning the data at that location into a managed struct without having to copy it).


So anyway, I looked at coredll.def from Platform Builder 5.0 to see if coredll.dll helps, and sure enough I see this:


malloc @1041
calloc @1346
_memccpy @1042
memcmp @1043
memcpy @1044
_memicmp @1045
memmove @1046
memset @1047


P/Invoke to the rescue.  I added the following to the SDF (so it will be in the next release).  Note that wile I was there I handles the annoyances of not having a way to copy from an IntPtr to an IntPtr (memcpy) or a way to validate IntPtrs (IsBad[Read/Write]Ptr) while I was at it.


public static void SetMemory(IntPtr destination, byte value, int length)
public static void SetMemory(IntPtr destination, byte value, int length, bool boundsCheck)
public static void Copy(IntPtr source, IntPtr destination, int length)
public static void Copy(IntPtr source, IntPtr destination, int length, bool boundsCheck)
public static bool IsSafeToWrite(IntPtr destination, int length)
public static bool IsSafeToRead(IntPtr source, int length)


Then a simple fix back in the ConnectionManager:


hDestInfo = Marshal.AllocHGlobal(DestinationInfo.NativeSize);
Marshal2.SetMemory(hDestInfo, 0, DestinationInfo.NativeSize, false);


 

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s