HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Class for RegQueryInfoKey pinvoke

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Scope of methods and const

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/2655508

So

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 like

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;
}
.....


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

null


or

"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.