Is this "unsafe" declaration unnecessary? I have a class that I inherited from someone else. It is doing a P/Invoke on CopyFileEx to copy a file (we are using UNC shares if that matters). The code is marked unsafe but I thought you only needed to use that when dealing with raw pointers, if you are using IntPtr only your code does not need to be marked unsafe.

public void Copy()
{
    bool result;

    unsafe
    {
        if (System.Environment.OSVersion.Version.Major &gt; 5)
        {
            //only newer versions of windows support the no buffer flag
            //(this is set because copying large files on a computer with limited ram will pretty much kill the computer)
            result = CopyFileEx(SourceFile, TargetFile, new CopyProgressRoutine(CopyProgressEvent), IntPtr.Zero, ref Cancelflag, CopyFileFlags.COPY_FILE_NO_BUFFERING);
        }
        else
        {
            result = CopyFileEx(SourceFile, TargetFile, new CopyProgressRoutine(CopyProgressEvent), IntPtr.Zero, ref Cancelflag, 0);
        }
    }


    //file copy failed so get the DLL error and throw an exception
    if (result == false)
    {
        Win32Exception exc = new Win32Exception(Marshal.GetLastWin32Error());
        throw exc;
    }
}


//xcopy file progress event handler
private CopyProgressResult CopyProgressEvent(Int64 TotalFileSize, Int64 TotalBytesTransferred, Int64 StreamSize, Int64 StreamBytesTransferred, uint dwStreamNumber,
                                                   XCopy.CopyProgressCallbackReason dwCallbackReason, IntPtr hSourceFile, IntPtr hDestinationFile, IntPtr lpData)
{
    //chunk of a file finished copying
    if (dwCallbackReason == XCopy.CopyProgressCallbackReason.CALLBACK_CHUNK_FINISHED)
    {
        int currentFilePercent = Convert.ToInt32(Math.Round(TotalBytesTransferred * 100.0 / TotalFileSize));

        //only send a new message when something changes (prevents flooding of messages)
        if (currentFilePercent != LastFilePercent)
        {
            LastFilePercent = currentFilePercent;

            //Report back the progress
            ProgressCallback.Report(currentFilePercent);
        }
    }

    //stop the copy?
    if (Cancelflag == 1)
        return XCopy.CopyProgressResult.PROGRESS_CANCEL;
    else
        return XCopy.CopyProgressResult.PROGRESS_CONTINUE;

}

//win32 API call
[DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
[return: MarshalAs(UnmanagedType.Bool)]
static extern bool CopyFileEx(string lpExistingFileName, string lpNewFileName,
    CopyProgressRoutine lpProgressRoutine, IntPtr lpData, ref Int32 pbCancel,
    CopyFileFlags dwCopyFlags);

//xcopy event handler
public delegate CopyProgressResult CopyProgressRoutine(
                    Int64 TotalFileSize,
                    Int64 TotalBytesTransferred,
                    Int64 StreamSize,
                    Int64 StreamBytesTransferred,
                    uint dwStreamNumber,
                    CopyProgressCallbackReason dwCallbackReason,
                    IntPtr hSourceFile,
                    IntPtr hDestinationFile,
                    IntPtr lpData);


I think the unsafe is not needed but the person who wrote this originally claims he was getting intermittent errors during run-time when in production environments when it was not marked unsafe.

Was he right about needing unsafe or did he have a problem elsewhere that was just fixed at the same time that made it look like unsafe fixed the issue?
unsafe is purely a compile-time check. If it were needed the code wouldn’t compile. The runtime errors are caused by something else.

Furthermore, unsafe is needed if and only if you are using unmanaged pointers (e.g. inside a fixed block).