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

Clean out Windows 9x code #154

Merged
merged 2 commits into from Dec 7, 2018

Conversation

Projects
None yet
4 participants
@JeremyKuhne
Member

JeremyKuhne commented Dec 5, 2018

As much as I fondly remember Windows 95/98 I don't think we need the code to support it any more. :)

This removes a large chunk of 9x specific code as well as some other dead/unused code in interop.

I have some other interop correctness and performance fixes that I'll follow this with.

@JeremyKuhne JeremyKuhne requested a review from dotnet/dotnet-winforms as a code owner Dec 5, 2018

@merriemcgaw

This comment has been minimized.

Member

merriemcgaw commented Dec 5, 2018

Love it! We're going to want to get our runtime functional regression suite online so we can make sure there are no surprises before taking this PR. Will keep you posted to know when it's ready.

@Tanya-Solyanik

LGTM, but we would want to run some sanity test manually before the merge. Thank you Jeremy! Looking forward to more PRs :)

if (unicode)
{
Marshal.Copy(new char[] { '\0' }, 0, currentPtr, 1);
// NOTE: DllLib.copy(char[]...) converts to ANSI on Windows 95...

This comment has been minimized.

@daniel-white

daniel-white Dec 5, 2018

is this this needed?

This comment has been minimized.

@JeremyKuhne

JeremyKuhne Dec 5, 2018

Member

If you mean the comment, it isn't. Since this method is suboptimal I wasn't super aggressive on the comment (or others like it) pending more aggressive cleanup/analysis. In this particular case I've already "fixed" the method, I'm just waiting on this higher level PR to go through. If you're curious, this is the improved version:

        private unsafe int SaveFileListToHandle(IntPtr handle, string[] files)
        {
            if (files == null || files.Length < 1)
                return NativeMethods.S_OK;
            if (handle == IntPtr.Zero)
                return NativeMethods.E_INVALIDARG;

            int sizeInBytes = sizeof(DROPFILES);

            // First determine the size of the array
            for (int i=0; i < files.Length; i++)
            {
                sizeInBytes += (files[i].Length + 1) * sizeof(char);
            }

            // Alloc the Win32 memory (on 9x we passed DDESHARE- NT ignores this)

            IntPtr newHandle = UnsafeNativeMethods.GlobalReAlloc(new HandleRef(null, handle), sizeInBytes, NativeMethods.GMEM_MOVEABLE);
            if (newHandle == IntPtr.Zero)
                return (NativeMethods.E_OUTOFMEMORY);

            void* basePointer = UnsafeNativeMethods.GlobalLock(new HandleRef(null, newHandle)).ToPointer();
            if (basePointer == null)
                return (NativeMethods.E_OUTOFMEMORY);

            try
            {
                // Write out the struct...
                DROPFILES* dropFiles = (DROPFILES*)basePointer;
                dropFiles->pFiles = (uint)sizeof(DROPFILES);
                dropFiles->pt = default;
                dropFiles->fNC = Interop.BOOL.FALSE;
                dropFiles->fWide = unchecked((Interop.BOOL)0xFFFFFFFF); // We set 0xFFFFFFFF for true historically

                Span<char> fileData = new Span<char>((void*)((byte*)basePointer + sizeof(DROPFILES)), sizeInBytes - sizeof(DROPFILES));

                // Write out the null terminated strings...
                foreach (string file in files)
                {
                    file.AsSpan().CopyTo(fileData);
                    fileData[file.Length] = '\0';
                    fileData = fileData.Slice(file.Length + 1);
                }

                // Write out the terminating double null
                fileData[0] = '\0';
            }
            finally
            {
                // Unlock the memory
                UnsafeNativeMethods.GlobalUnlock(new HandleRef(null, newHandle));
            }

            return NativeMethods.S_OK;
        }

@merriemcgaw merriemcgaw merged commit 0bfb389 into dotnet:master Dec 7, 2018

1 check passed

license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment