patterncsharpMinor
Expecting working sample of DeviceIoControl reviewed
Viewed 0 times
samplerevieweddeviceiocontrolworkingexpecting
Problem
I put the full code at the rear of this post.
I've recently answered a question, and the detail explanation is posted in
And en passant answer another one
I'd like to know that if any one think there's any problem or improvement of the code.
code:
```
using System.Runtime.InteropServices;
using System.ComponentModel;
using System;
namespace DiskManagement {
using Microsoft.Win32.SafeHandles;
using LPSECURITY_ATTRIBUTES=IntPtr;
using LPOVERLAPPED=IntPtr;
using LPVOID=IntPtr;
using HANDLE=IntPtr;
using LARGE_INTEGER=Int64;
using DWORD=UInt32;
using LPCTSTR=String;
public static partial class IoCtl / methods / {
[DllImport("kernel32.dll", SetLastError=true)]
static extern SafeFileHandle CreateFile(
LPCTSTR lpFileName,
DWORD dwDesiredAccess,
DWORD dwShareMode,
LPSECURITY_ATTRIBUTES lpSecurityAttributes,
DWORD dwCreationDisposition,
DWORD dwFlagsAndAttributes,
HANDLE hTemplateFile
);
[DllImport("kernel32.dll", SetLastError=true)]
static extern DWORD DeviceIoControl(
SafeFileHandle hDevice,
DWORD dwIoControlCode,
LPVOID lpInBuffer,
DWORD nInBufferSize,
LPVOID lpOutBuffer,
int nOutBufferSize,
ref DWORD lpBytesReturned,
LPOVERLAPPED lpOverlapped
);
static DWORD CTL_CODE(DWORD DeviceType, DWORD Function, DWORD Method, DWORD Access) {
return (((DeviceType)(
ref T x,
DWORD dwIoControlCode,
LPCTSTR lpFileName,
DWORD dwDesiredAccess=GENERIC_READ,
DWORD dwShareMode=FILE_SHARE_WRITE|FILE_SHARE_READ,
LPSECURITY_ATTRIBUTES lpSecurityAttributes=default(LPSECURITY_ATTRIBUTES),
DWORD dwCrea
I've recently answered a question, and the detail explanation is posted in
- Physical disk size not correct (IoCtlDiskGetDriveGeometry)
And en passant answer another one
- DeviceIoControl does not set output buffer
I'd like to know that if any one think there's any problem or improvement of the code.
code:
```
using System.Runtime.InteropServices;
using System.ComponentModel;
using System;
namespace DiskManagement {
using Microsoft.Win32.SafeHandles;
using LPSECURITY_ATTRIBUTES=IntPtr;
using LPOVERLAPPED=IntPtr;
using LPVOID=IntPtr;
using HANDLE=IntPtr;
using LARGE_INTEGER=Int64;
using DWORD=UInt32;
using LPCTSTR=String;
public static partial class IoCtl / methods / {
[DllImport("kernel32.dll", SetLastError=true)]
static extern SafeFileHandle CreateFile(
LPCTSTR lpFileName,
DWORD dwDesiredAccess,
DWORD dwShareMode,
LPSECURITY_ATTRIBUTES lpSecurityAttributes,
DWORD dwCreationDisposition,
DWORD dwFlagsAndAttributes,
HANDLE hTemplateFile
);
[DllImport("kernel32.dll", SetLastError=true)]
static extern DWORD DeviceIoControl(
SafeFileHandle hDevice,
DWORD dwIoControlCode,
LPVOID lpInBuffer,
DWORD nInBufferSize,
LPVOID lpOutBuffer,
int nOutBufferSize,
ref DWORD lpBytesReturned,
LPOVERLAPPED lpOverlapped
);
static DWORD CTL_CODE(DWORD DeviceType, DWORD Function, DWORD Method, DWORD Access) {
return (((DeviceType)(
ref T x,
DWORD dwIoControlCode,
LPCTSTR lpFileName,
DWORD dwDesiredAccess=GENERIC_READ,
DWORD dwShareMode=FILE_SHARE_WRITE|FILE_SHARE_READ,
LPSECURITY_ATTRIBUTES lpSecurityAttributes=default(LPSECURITY_ATTRIBUTES),
DWORD dwCrea
Solution
As most people in the comments suggest, it is missing a few conventions that are pretty generally accepting across all C# developers. This will mostly be looking at stylistic points as there isn't really much in wrong with the implementation.
General notes
Most of the bracket/whitespace-related points are enforced by Visual Studio when you close a code block (
-
Your spacing is a bit off, all curly braces should be on separate lines. Methods generally should look like this:
-
Generally you should have 1 (partial) class per file, this may not be applicable but wanted to mention.
-
Order your type's members like this:
-
Normally
Line-by-line
Normally comments are made on the line above class definitions, you may be better putting this into an IoCtrl.Methods.cs file or something if you really want to separate the different sections of the file. I usually stay away from
While I'm not particularly a fan of it, your 'defines' at the top make sense given their application and the documentation of the kernal32.dll assembly.
Structure your braces like this:
Try not to span too many lines when calling functions
Using
Always prefer
Use pascal case for all types.
Always specify access modifiers.
There isn't really a standard on this but I prefer to keep simple getters on a single line. Also keep variables at the top of the file and use
```
// properties and fields
partial class DiskGeometry
{
private CubicAddress maximumCub
General notes
Most of the bracket/whitespace-related points are enforced by Visual Studio when you close a code block (
{).- Types should be in
PascalCase
- Public member names should be in
PascalCase
- Hungarian notation (
dwDesiredAccess) is discourages except for when naming controls in WebForms and WinForms. Go withcamelCase.
-
Your spacing is a bit off, all curly braces should be on separate lines. Methods generally should look like this:
public void Method(Type param)
{
// Implementation
}-
Generally you should have 1 (partial) class per file, this may not be applicable but wanted to mention.
- Placing a space between most symbols is encourages.
-
Order your type's members like this:
- Variables (pref. const/static first)
- Constructors/Properties (either way)
- Methods
-
Normally
ALL_CAPS-style naming is not used in C#, sometimes it is for constants but generally not.Line-by-line
Normally comments are made on the line above class definitions, you may be better putting this into an IoCtrl.Methods.cs file or something if you really want to separate the different sections of the file. I usually stay away from
partial unless the file gets ridiculously long and can't be factored out.// methods
public static partial class IoCtl {While I'm not particularly a fan of it, your 'defines' at the top make sense given their application and the documentation of the kernal32.dll assembly.
using LPSECURITY_ATTRIBUTES = IntPtr;
using LPOVERLAPPED = IntPtr;
using LPVOID = IntPtr;
using HANDLE = IntPtr;
using LARGE_INTEGER = Int64;
using DWORD = UInt32;
using LPCTSTR = String;Structure your braces like this:
public static void Execute(
ref T x,
DWORD dwIoControlCode,
LPCTSTR lpFileName,
DWORD dwDesiredAccess = GENERIC_READ,
DWORD dwShareMode = FILE_SHARE_WRITE | FILE_SHARE_READ,
LPSECURITY_ATTRIBUTES lpSecurityAttributes = default(LPSECURITY_ATTRIBUTES),
DWORD dwCreationDisposition = OPEN_EXISTING,
DWORD dwFlagsAndAttributes = 0,
HANDLE hTemplateFile = default(IntPtr))
{Try not to span too many lines when calling functions
using (var hDevice = CreateFile(lpFileName, dwDesiredAccess, dwShareMode,
lpSecurityAttributes, dwCreationDisposition, dwFlagsAndAttributes,
hTemplateFile))
{
if (null == hDevice || hDevice.IsInvalid)
throw new Win32Exception(Marshal.GetLastWin32Error());
var nOutBufferSize = Marshal.SizeOf(typeof(T));
var lpOutBuffer = Marshal.AllocHGlobal(nOutBufferSize);
var lpBytesReturned = default(DWORD);Using
IntPtr.Zero rather than an alias (NULL) makes it easier to understand what the value is later on.var result = DeviceIoControl(hDevice, dwIoControlCode, IntPtr.Zero, 0,
lpOutBuffer, nOutBufferSize, ref lpBytesReturned, IntPtr.Zero);Always prefer
var == over == var, this type of condition is affectionately known as a 'yoda condition'.if (result == 0)
throw new Win32Exception(Marshal.GetLastWin32Error());
x = (T)Marshal.PtrToStructure(lpOutBuffer, typeof(T));
Marshal.FreeHGlobal(lpOutBuffer);
}
}Use pascal case for all types.
public enum MediaType : int
{
Unknown = 0,
F5_1Pt2_512 = 1,
F3_1Pt44_512 = 2,
F3_2Pt88_512 = 3,
F3_20Pt8_512 = 4,
F3_720_512 = 5,
F5_360_512 = 6,
F5_320_512 = 7,
F5_320_1024 = 8,
F5_180_512 = 9,
F5_160_512 = 10,
RemovableMedia = 11,
FixedMedia = 12,
F3_120M_512 = 13,
F3_640_512 = 14,
F5_640_512 = 15,
F5_720_512 = 16,
F3_1Pt2_512 = 17,
F3_1Pt23_1024 = 18,
F5_1Pt23_1024 = 19,
F3_128Mb_512 = 20,
F3_230Mb_512 = 21,
F8_256_128 = 22,
F3_200Mb_512 = 23,
F3_240M_512 = 24,
F3_32M_512 = 25
}Always specify access modifiers.
// Structures
internal partial class DiskGeometry
{
[StructLayout(LayoutKind.Sequential)]
internal struct DISK_GEOMETRY
{
internal LARGE_INTEGER Cylinders;
internal MEDIA_TYPE MediaType;
internal DWORD TracksPerCylinder;
internal DWORD SectorsPerTrack;
internal DWORD BytesPerSector;
}
[StructLayout(LayoutKind.Sequential)]
internal struct DISK_GEOMETRY_EX
{
internal DISK_GEOMETRY Geometry;
internal LARGE_INTEGER DiskSize;
[MarshalAs(UnmanagedType.ByValArray, SizeConst=1)]
internal byte[] Data;
}
}There isn't really a standard on this but I prefer to keep simple getters on a single line. Also keep variables at the top of the file and use
pascalCase;```
// properties and fields
partial class DiskGeometry
{
private CubicAddress maximumCub
Code Snippets
public void Method(Type param)
{
// Implementation
}// methods
public static partial class IoCtl {using LPSECURITY_ATTRIBUTES = IntPtr;
using LPOVERLAPPED = IntPtr;
using LPVOID = IntPtr;
using HANDLE = IntPtr;
using LARGE_INTEGER = Int64;
using DWORD = UInt32;
using LPCTSTR = String;public static void Execute<T>(
ref T x,
DWORD dwIoControlCode,
LPCTSTR lpFileName,
DWORD dwDesiredAccess = GENERIC_READ,
DWORD dwShareMode = FILE_SHARE_WRITE | FILE_SHARE_READ,
LPSECURITY_ATTRIBUTES lpSecurityAttributes = default(LPSECURITY_ATTRIBUTES),
DWORD dwCreationDisposition = OPEN_EXISTING,
DWORD dwFlagsAndAttributes = 0,
HANDLE hTemplateFile = default(IntPtr))
{using (var hDevice = CreateFile(lpFileName, dwDesiredAccess, dwShareMode,
lpSecurityAttributes, dwCreationDisposition, dwFlagsAndAttributes,
hTemplateFile))
{
if (null == hDevice || hDevice.IsInvalid)
throw new Win32Exception(Marshal.GetLastWin32Error());
var nOutBufferSize = Marshal.SizeOf(typeof(T));
var lpOutBuffer = Marshal.AllocHGlobal(nOutBufferSize);
var lpBytesReturned = default(DWORD);Context
StackExchange Code Review Q#23264, answer score: 4
Revisions (0)
No revisions yet.