patterncsharpMinor
Class for RegQueryInfoKey pinvoke
Viewed 0 times
pinvokeclassforregqueryinfokey
Problem
I'd like to make sure I'm following some basic coding best practices since I've had only little practice creating my own classes and using pinvoke. I've found this class to be pretty useful and I'd like to put it up online but not if it's a poorly written or could cause issues I haven't considered.
I would love any feedback or suggestions for things I should review/revise or completely redo.
```
///
/// Description: Class for the use of RegQueryInfoKey pinvoke interop in C#.
/// Usage Example: DateTime dTime = RegQuery.lastWriteTime("HKEY_Current_user\\software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Dropbox");
///
using System;
using System.Runtime.InteropServices;
using System.Text;
namespace WindowsForm
{
public class RegQuery
{
public const int KEY_QUERY_VALUE = 0x1;
static UIntPtr hKey = (UIntPtr)0x80000002;
static UIntPtr hKeyVal;
static StringBuilder classStr = new StringBuilder(255);
static uint classSize = (uint)classStr.Capacity + 1;
static uint lpcSubKeys;
static uint lpcbMaxSubKeyLen;
static uint lpcbMaxClassLen;
static uint lpcValues;
static uint lpcbMaxValueNameLen;
static uint lpcbMaxValueLen;
static uint lpcbSecurityDescriptor;
static long lpftLastWriteTime;
[DllImport("advapi32.dll", EntryPoint = "RegOpenKeyEx")]
extern private static int RegOpenKeyEx_DllImport(UIntPtr hKey, string lpSubKey, uint ulOptions, int samDesired, out UIntPtr phkResult);
[DllImport("advapi32.dll")]
extern private static int RegQueryInfoKey(
UIntPtr hkey,
StringBuilder lpClass,
ref uint lpcbClass,
IntPtr lpReserved,
out uint lpcSubKeys,
out uint lpcbMaxSubKeyLen,
out uint lpcbMaxClassLen,
out uint lpcValues,
out uint lpcbMaxValueNameLen,
out uint lpcbMaxValueLen,
out uint lpcbSecurityD
I would love any feedback or suggestions for things I should review/revise or completely redo.
```
///
/// Description: Class for the use of RegQueryInfoKey pinvoke interop in C#.
/// Usage Example: DateTime dTime = RegQuery.lastWriteTime("HKEY_Current_user\\software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Dropbox");
///
using System;
using System.Runtime.InteropServices;
using System.Text;
namespace WindowsForm
{
public class RegQuery
{
public const int KEY_QUERY_VALUE = 0x1;
static UIntPtr hKey = (UIntPtr)0x80000002;
static UIntPtr hKeyVal;
static StringBuilder classStr = new StringBuilder(255);
static uint classSize = (uint)classStr.Capacity + 1;
static uint lpcSubKeys;
static uint lpcbMaxSubKeyLen;
static uint lpcbMaxClassLen;
static uint lpcValues;
static uint lpcbMaxValueNameLen;
static uint lpcbMaxValueLen;
static uint lpcbSecurityDescriptor;
static long lpftLastWriteTime;
[DllImport("advapi32.dll", EntryPoint = "RegOpenKeyEx")]
extern private static int RegOpenKeyEx_DllImport(UIntPtr hKey, string lpSubKey, uint ulOptions, int samDesired, out UIntPtr phkResult);
[DllImport("advapi32.dll")]
extern private static int RegQueryInfoKey(
UIntPtr hkey,
StringBuilder lpClass,
ref uint lpcbClass,
IntPtr lpReserved,
out uint lpcSubKeys,
out uint lpcbMaxSubKeyLen,
out uint lpcbMaxClassLen,
out uint lpcValues,
out uint lpcbMaxValueNameLen,
out uint lpcbMaxValueLen,
out uint lpcbSecurityD
Solution
Scope of methods and const
There is no reason for
Naming
Following the naming guidlines you should use
So
will become ( with casing and scope )
and
will become
but this is more named like a property. A method should be named as a verb or a verb phrases. It also reads in the comments to this method
Braces
It is best practice to use braces for if..else, for.. etc. everytime, also if it would not be neccessary. See: https://codereview.stackexchange.com/a/49212/29371
So the
Also it is only a matter of taste, i prefer
Validation
You should validate the inputparameter of the public methods for correctness. Assume a user of this class will call one of these methods with a parameter like
or
or
GetLastError
While pinvoking native dlls, you should always check, if the call to one of the native methods,results in an error.
You can do this by setting the attribute
and calling Marshall.GetLastWin32Error . See also: https://stackoverflow.com/a/17918729/2655508
There is no reason for
KEY_QUERY_VALUE to has a public scope. The same is true for the doQuery() method.Naming
Following the naming guidlines you should use
PascalCasing for names of classes, structs and methods. See also: https://stackoverflow.com/a/1618325/2655508So
public static void doQuery(string fullKey)will become ( with casing and scope )
private static void DoQuery(string fullKey)and
public static uint subKeys(string fullKey)will become
public static uint SubKeys(string fullKey)but this is more named like a property. A method should be named as a verb or a verb phrases. It also reads in the comments to this method
...that receives the number of subkeys... , so a better name would be GetNumberOfSubKeys. This is also true for your other methodnames.Braces
It is best practice to use braces for if..else, for.. etc. everytime, also if it would not be neccessary. See: https://codereview.stackexchange.com/a/49212/29371
So the
if..else construct in the DoQuery() method would look likeif (String.Equals(hive[0], "HKEY_LOCAL_MACHINE", StringComparison.OrdinalIgnoreCase) || String.Equals(hive[0], "HKLM", StringComparison.OrdinalIgnoreCase))
{
hKey = (UIntPtr)0x80000002;
}
else if (String.Equals(hive[0], "HKEY_CURRENT_USER", StringComparison.OrdinalIgnoreCase) || String.Equals(hive[0], "HKCU", StringComparison.OrdinalIgnoreCase))
{
hKey = (UIntPtr)0x80000001;
}
.....Also it is only a matter of taste, i prefer
switch..case over if..else if .Validation
You should validate the inputparameter of the public methods for correctness. Assume a user of this class will call one of these methods with a parameter like
"\\HKEY_Current_user\\software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Dropbox"or
nullor
"HKEYY_Current_user\\software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Dropbox"GetLastError
While pinvoking native dlls, you should always check, if the call to one of the native methods,results in an error.
You can do this by setting the attribute
SetLastError=true like[DllImport("advapi32.dll",, SetLastError=true, EntryPoint = "RegOpenKeyEx")]
extern private static int RegOpenKeyEx_DllImport(UIntPtr hKey, string lpSubKey,
uint ulOptions, int samDesired, out UIntPtr phkResult);and calling Marshall.GetLastWin32Error . See also: https://stackoverflow.com/a/17918729/2655508
Code Snippets
public static void doQuery(string fullKey)private static void DoQuery(string fullKey)public static uint subKeys(string fullKey)public static uint SubKeys(string fullKey)if (String.Equals(hive[0], "HKEY_LOCAL_MACHINE", StringComparison.OrdinalIgnoreCase) || String.Equals(hive[0], "HKLM", StringComparison.OrdinalIgnoreCase))
{
hKey = (UIntPtr)0x80000002;
}
else if (String.Equals(hive[0], "HKEY_CURRENT_USER", StringComparison.OrdinalIgnoreCase) || String.Equals(hive[0], "HKCU", StringComparison.OrdinalIgnoreCase))
{
hKey = (UIntPtr)0x80000001;
}
.....Context
StackExchange Code Review Q#58454, answer score: 4
Revisions (0)
No revisions yet.