Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to get dbcc_name from DEV_BROADCAST_DEVICEINTERFACE_W #1005

Closed
a4a6cb31 opened this issue Jul 31, 2023 · 6 comments
Closed

Unable to get dbcc_name from DEV_BROADCAST_DEVICEINTERFACE_W #1005

a4a6cb31 opened this issue Jul 31, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@a4a6cb31
Copy link

Actual behavior

The generated struct includes the dbcc_name field as winmdroot.__char_1. It should be a null terminated string, but there's only ever one character there.

Expected behavior

I expect a char* with my full string available.

Repro steps

  1. NativeMethods.txt content:
RegisterDeviceNotification
DEV_BROADCAST_HDR
DEV_BROADCAST_DEVICEINTERFACE_W
WM_DEVICECHANGE
DBT_DEVICEARRIVAL
  1. Any of your own code that should be shared?

Here's a quick WinForms example that should repro the issue. Plug in a flash drive with this running.

    public unsafe partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();

            DEV_BROADCAST_HDR dbi = new()
            {
                dbch_size = (uint)Marshal.SizeOf<DEV_BROADCAST_HDR>(),
                dbch_devicetype = DEV_BROADCAST_HDR_DEVICE_TYPE.DBT_DEVTYP_DEVICEINTERFACE
            };

            PInvoke.RegisterDeviceNotification((HANDLE)Handle, &dbi, REGISTER_NOTIFICATION_FLAGS.DEVICE_NOTIFY_ALL_INTERFACE_CLASSES);
        }

        protected override void WndProc(ref Message m)
        {
            if (m.Msg == PInvoke.WM_DEVICECHANGE && m.WParam.ToInt64() == PInvoke.DBT_DEVICEARRIVAL)
            {
                DEV_BROADCAST_DEVICEINTERFACE_W deviceInterface = Marshal.PtrToStructure<DEV_BROADCAST_DEVICEINTERFACE_W>(m.LParam);
                string badData = deviceInterface.dbcc_name.ToString();
            }

            base.WndProc(ref m);
        }
    }

If I declare my own struct as follows, the above code then works as I expect.

    [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
    internal unsafe struct DEV_BROADCAST_DEVICEINTERFACE
    {
        public uint dbcc_size;
        public uint dbcc_devicetype;
        public uint dbcc_reserved;
        public Guid dbcc_classguid;
        public fixed char dbcc_name[255];
    }

Context

  • CsWin32 version: 0.3.18-beta
  • Win32Metadata version (if explicitly set by project):
  • Target Framework: netstandard2.0
  • LangVersion: latest
@a4a6cb31 a4a6cb31 added the bug Something isn't working label Jul 31, 2023
@AArnott
Copy link
Member

AArnott commented Aug 11, 2023

It won't be a char*, since it's an inline array. But you're right that this is an issue.
Duplicate of #387.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Aug 11, 2023
@Ashfaaq18
Copy link

Ashfaaq18 commented Feb 10, 2024

Hi @AArnott , I'm not sure if this is fully fixed. I'm working with some USB event notification code and I use the CsWin32 library as follows,

    case PInvoke.WM_DEVICECHANGE:
        {
            switch ((uint)wParam)
            {
                case PInvoke.DBT_DEVICEARRIVAL:
                    {
                        if (lParam != nint.Zero)
                        {
                            DEV_BROADCAST_HDR hdr = Marshal.PtrToStructure<DEV_BROADCAST_HDR>(lParam);
                            if (hdr.dbch_devicetype == DEV_BROADCAST_HDR_DEVICE_TYPE.DBT_DEVTYP_DEVICEINTERFACE)
                            {
                                DEV_BROADCAST_DEVICEINTERFACE_W devIntW = Marshal.PtrToStructure<DEV_BROADCAST_DEVICEINTERFACE_W>(lParam);
                                if (devIntW.dbcc_classguid.Equals(PInvoke.GUID_DEVINTERFACE_USB_DEVICE))
                                {
                                    unsafe
                                    {
                                        // This outputs => "\\"
                                        string name = new string((char*)&devIntW.dbcc_name);

                                        var address = nint.Add((nint)(&devIntW.dbcc_name), 1184);
                                        // This outputs => "\\?\USB#VID_18A5&PID_0243#07010A63CCD74E16#{a5dcbf10-6530-11d2-901f-00c04fb951ed}"
                                        //I looked around the memory of this address and found the dbcc_name far away from where it should be.
                                        //An offset of 1184 gets me the USB id
                                        name = new string((char*)address);
                                    }
                                }
                            }
                        }
                    }
                    break;

I have some more code where I extract the SP_DEVICE_INTERFACE_DETAIL_DATA_W's __char_1 DevicePath and return the null terminated string as " return new string((char*)&pD->DevicePath); " without a problem. I am lost as to why i should offset the this dbcc_name by 1184 to obtain the device_name. Any help/tips? is it something to do with the generated struct?

@BartoszCichecki
Copy link

BartoszCichecki commented May 12, 2024

I am seeing exactly the same issue. The name is waaaay further in memory than it should be. Here is the workaround I am using:

do not use this hack

This is with cswin32 0.3.106.

@AArnott
Copy link
Member

AArnott commented May 13, 2024

The OP code was faulty. Using Marshal.PtrToStructure is inappropriate because the pointer given in LParam is a pointer to a buffer that starts with the struct but the struct does not define all the data, so PtrToStructure has no idea to copy the actual text of the device name. It is important to access the data where it originally is.

I tested this modified code and found it works:

protected override void WndProc(ref Message m)
{
    if (m.Msg == PInvoke.WM_DEVICECHANGE && m.WParam.ToInt64() == PInvoke.DBT_DEVICEARRIVAL)
    {
        ref DEV_BROADCAST_DEVICEINTERFACE_W deviceInterface = ref Unsafe.AsRef<DEV_BROADCAST_DEVICEINTERFACE_W>((void*)m.LParam);
        // The length of the entire structure (including trailing buffer) is given by dbcc_size.
        // We subtract the known size of the structure to get the length of the trailing buffer.
        // Then because the length is given in bytes, we divide by the size of a char to get the number of characters.
        int length = ((int)deviceInterface.dbcc_size - sizeof(DEV_BROADCAST_DEVICEINTERFACE_W)) / sizeof(char);
        string? name = deviceInterface.dbcc_name.AsSpan(length).ToString();
    }

    base.WndProc(ref m);
}

@BartoszCichecki, Your workaround seems extremely dangerous and you evidently are just scanning random memory. You shouldn't make any assumptions about the length of the string or where in memory it might have been copied to. The above code I provide seems to work precisely.

@BartoszCichecki
Copy link

BartoszCichecki commented May 13, 2024

Thanks for the correct solution! You are right this was a very dangerous hack.

By the way, would it make sense to generate convenience methods for stuff like this?

@AArnott
Copy link
Member

AArnott commented May 13, 2024

I'm glad the solution works for you.

The VariableLengthInlineArray<T> type that CsWin32 emits is the best thing we have come up with so far to make this more convenient. The challenge is that there isn't a single consistent pattern for how native inline arrays are used, defined, sized, etc. We don't have the time budget to add special case convenience APIs for every win32 API, so we mainly focus on general patterns that we can apply to make things easier.
FWIW, I don't think what we require users to do with our general APIs is worse than a C programmer. And our scope with CsWin32 is not to make interop easier in C# than C. We merely aim to expose the APIs so you don't have to write them yourself. But reading the API docs and carefully managing memory as necessary is still an exercise left to the reader.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants