sharpdx / SharpDX

SharpDX GitHub Repository
http://sharpdx.org
MIT License
1.69k stars 638 forks source link

Adapter Description text is not zero terminated any more because of invalid Utilities.PtrToStringUni #828

Closed abenedik closed 7 years ago

abenedik commented 7 years ago

I use AdapterDescription1.Description text to show the used Adapter.

When using SharpDX 2.6.3 this worked well. But in SharpDX 3.1 the Description text is 128 chars long and contains the actual adapter description followed with many zero characters.

The AdapterDescription1.Description text is initialized by using the:

fixed (char* chPtr = &@ref.Description)
    Description = Utilities.PtrToStringUni((IntPtr) ((void*) chPtr), 128);

The problem is that the Utilities.PtrToStringUni has changed. In the 2.6.3 version the method was implemented as:

/// <summary>
/// Converts a pointer to a null-terminating string up to maxLength characters to a .Net string.
/// </summary>
/// <param name="pointer">The pointer to an Unicode null string.</param>
/// <param name="maxLength">Maximum length of the string.</param>
/// <returns>The converted string.</returns>
public static string PtrToStringUni(IntPtr pointer, int maxLength)
{
#if W8CORE
    return Marshal.PtrToStringUni(pointer, maxLength);
#else
    unsafe
    {
        var pStr = (char*)pointer;
        for (int i = 0; i < maxLength; i++)
            if (*pStr++ == 0)
                return new string((char*)pointer);
        return new string((char*)pointer, 0, maxLength);
    }
#endif
} 

The unsafe part of the code correctly ended the .Net string at first '\0' char. The

description of the method shows that this is the desired functionality.

But in the latest version of SharpDX the source for the PtrToStringUni is simply:

public static string PtrToStringUni(IntPtr pointer, int maxLength)
{
    return Marshal.PtrToStringUni(pointer, maxLength);
} 

As written in the MSDN description (https://msdn.microsoft.com/en-us/library/k4x4bfws(v=vs.110).aspx). the Marshal.PtrToStringUni just copies the specified number of unmanaged characters into managed string and does not check for the first zero char.

I think that this method should be reverted to its previous version.

Another option is to use Marshal.PtrToStringUni method without specifying the maxLenght - this will return the string when the first zero char is found (though this does not allow limiting the string to 128 chars) - see https://msdn.microsoft.com/en-us/library/dyf0tzy4(v=vs.110).aspx

A quick search reveals that the PtrToStringUni is used for Description that is defined in AdapterDescription, AdapterDescription1, AdapterDescription2 and in DeviceName defined in OutputDescription.

xoofx commented 7 years ago

Yeah, I guess what happened is that at some point the constructor new string((char*) was not supported on one platform (Windows/WindowsPhone 8 certification that was really horrible to work with...) and when the code was removed to try to make things more uniform, the original behaviour was actually not preserved