Could you safely say this code is "unsafe"

Question

this is the calss that implements native pinvok code

Code That seem to work though i could not verify it is using unsafe properly

signeture with unsafe

    struct IO_COUNTERS
    {
        public ulong ReadOperationCount;
        public ulong WriteOperationCount;
        public ulong OtherOperationCount;
        public ulong ReadTransferCount;
        public ulong WriteTransferCount;
        public ulong OtherTransferCount;
    }
    [DllImport("kernel32.dll")]
    unsafe static extern bool GetProcessIoCounters(IntPtr* ProcessHandle, out IO_COUNTERS IoCounters);

code with my "unsafe"

        public static unsafe class IO
        {
            public static Dictionary<string, ulong> GetALLIO(Process procToRtrivIO)
            {
                IO_COUNTERS counters;
                Dictionary<string, ulong> retCountIoDict = new Dictionary<string, ulong>();
                GetProcessIoCounters((IntPtr*)System.Diagnostics.Process.GetCurrentProcess().Handle, out counters);
                retCountIoDict.Add("ReadOperationCount", counters.ReadOperationCount);
                retCountIoDict.Add("WriteOperationCount", counters.WriteOperationCount);
                retCountIoDict.Add("OtherOperationCount", counters.OtherOperationCount);
                retCountIoDict.Add("ReadTransferCount", counters.ReadTransferCount);
                retCountIoDict.Add("WriteTransferCount", counters.WriteTransferCount);
                retCountIoDict.Add("OtherTransferCount", counters.OtherTransferCount);
                return retCountIoDict;
                //return  "This process has read " + ((counters.ReadTransferCount/1024)/1024).ToString("N0") +
                //    " Mb of data.";

            }
        }

now this next block of code, was suggested in my other question (just before i managed to compile and run the code above, ...as i had some issues with it )

so just after i have solevd my question through trial and error someone posted this partial solution, and it is parital cause it does not include both struct and signeture, so i was asking if it was being tested cause it does compile but does not run . i mean when it runs(with no compliation error) but on run time, the error is

An exception of type 'System.NullReferenceException' occurred in App_Code.1ri3mog1.dll but was not handled in user code

Additional information: Object reference not set to an instance of an object.

so this is the code i got from my last question

Ok you need to change a few things, shown below

public static class IO
{
    unsafe public static Dictionary<string, ulong> GetALLIO(Process procToRtrivIO)
    {
        IO_COUNTERS* counters;
        Dictionary<string, ulong> retCountIoDict = new Dictionary<string, ulong>();
        IntPtr ptr = System.Diagnostics.Process.GetCurrentProcess().Handle;

        GetProcessIoCounters(&ptr, out counters);
        retCountIoDict.Add("ReadOperationCount", counters->ReadOperationCount);
        retCountIoDict.Add("WriteOperationCount", counters->WriteOperationCount);
        retCountIoDict.Add("OtherOperationCount", counters->OtherOperationCount);
        retCountIoDict.Add("ReadTransferCount", counters->ReadTransferCount);
        retCountIoDict.Add("WriteTransferCount", counters->WriteTransferCount);
        retCountIoDict.Add("OtherTransferCount", counters->OtherTransferCount);
        return retCountIoDict;
        //return  "This process has read " + ((counters.ReadTransferCount/1024)/1024).ToString("N0") +
        //    " Mb of data.";

    }
}

so question is , did i truely make it... and safely used an unsafe code (the top block of code ) or i have missed something, and if my code is ok(still i suspect it is not fully using unsafe fitures) , what is wrong with the suggested version of code ?

David Heffernan · Accepted Answer · 2013-06-20T12:01:43.267

2

You code is badly broken. Your declaration of GetProcessIoCounters is wrong. You are passing the address of the process handle but you need to pass the process handle by value. What's more there is no need at all for unsafe code here. I suggest you stop using unsafe.

Here's how you declare GetProcessIoCounters:

[DllImport(@"kernel32.dll", SetLastError=true)]
static extern bool GetProcessIoCounters(
    IntPtr hProcess, 
    out IO_COUNTERS IoCounters
);

Your declaration of IO_COUNTERS is fine.

To call the function you simply do this:

IO_COUNTERS IoCounters;
if (!GetProcessIoCounters(Process.GetCurrentProcess().Handle, out IoCounters))
    throw new Win32Exception();

edited Jun 20 '13 at 12:01

answered Jun 20 '13 at 11:32

David Heffernan

601,492
42
1,072
1,490

hey thanks for the code cause i realy forgot how it was without the unsafe already , though the task was not to use the .net own classes to aquire that info, so i could gain some performance atleast test if and how much did i get out of this p/invoke implementation , then i said to my self, ok let's go even further and test p/invoke plus unsafe then just p/invoke and back to .net cause i might use p/invoke if it will show some good performance gain but i am not sure as to using unsafe on regular bases , though just for my test i was trying to . – LoneXcoder Jun 20 '13 at 11:51
Using unsafe won't make this code faster. There's no point in using unsafe here. – David Heffernan Jun 20 '13 at 11:55
ok then just add the "bool" in signature and it's a signed deal (: – LoneXcoder Jun 20 '13 at 11:59
yes it happens when your brains is quicker than your fingers by a small margin (: i guess i am far from being a programmer but i could atleast be a good typo qa..(: thanks alot for stopping by . – LoneXcoder Jun 20 '13 at 12:10

Could you safely say this code is "unsafe"

1 Answers1