Struggling with some code.

Hi.

Could someone look over the following, it’s causing my program to freeze up and subsequently because the program is not responding windows itself.

BOOLEAN Registry(
HKEY hKey, /* hKey. */
LPCTSTR KeyName, /* Either the Path Name for OPENKEY, or the KeyName for entry access. */
PHKEY phKey, /* phKey. */
PWCHAR pBuffer, /* Return and save values from and in WCHAR. */
PDWORD dwBuffer, /* Return the DWORD value. */
DWORD MaximumCharLength, /* Maximum Length of pBuffer for saving. */
DWORD RegistryType) /* Registry Type. */
{
DWORD dwLocalLength;
LPBYTE pbBuffer;
PUCHAR uzBuffer;
DWORD dwRegType;
DWORD dwResult;
DWORD dwInt;

if(pBuffer == NULL) dwLocalLength = sizeof(DWORD); else dwLocalLength = MaximumCharLength;

dwResult = 0;
pbBuffer = (LPBYTE) malloc (dwLocalLength);
uzBuffer = (LPBYTE) malloc (11); /* Maximum DWORD length in characters is 10. */

switch(RegistryType)
{
case OPENKEY:
if (RegOpenKeyEx(hKey, KeyName, 0, KEY_ALL_ACCESS, phKey) != ERROR_SUCCESS)
if (RegCreateKeyEx(hKey, KeyName, 0, NULL, REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, phKey, &dwLocalLength) != ERROR_SUCCESS) break;
else if (RegOpenKeyEx(hKey, KeyName, 0, KEY_ALL_ACCESS, phKey) != ERROR_SUCCESS) break;
free(uzBuffer);
free(pbBuffer);
return TRUE;
case DGETKEY:
if (pBuffer != NULL) break;
if ((dwResult = RegQueryValueEx ((HKEY) *phKey, KeyName, 0, &dwRegType, (LPBYTE) &pbBuffer, &dwLocalLength)) != ERROR_SUCCESS) break;
if (StringCchPrintfA(uzBuffer, 11, “%d”, pbBuffer) != S_OK) break;
*dwBuffer = atoi(uzBuffer);
free(uzBuffer);
free(pbBuffer);
return TRUE;
case DSETKEY:
if (pBuffer != NULL) break;
if ((dwResult = RegSetValueEx ((HKEY) *phKey, KeyName, 0, REG_DWORD, (PBYTE) dwBuffer, sizeof(DWORD))) != ERROR_SUCCESS) break;
free(uzBuffer);
free(pbBuffer);
return TRUE;
case BGETKEY:
if (dwBuffer != NULL) break;
if ((dwResult = RegQueryValueEx ((HKEY) *phKey, KeyName, 0, &dwRegType, (LPBYTE) pbBuffer, &dwLocalLength)) != ERROR_SUCCESS) break;
if (StringCchPrintfW(pBuffer, MaximumCharLength, L"%ws", pbBuffer) != S_OK) break;
free(uzBuffer);
free(pbBuffer);
return TRUE;
case BSETKEY:
if (dwBuffer != NULL) break;
if ((dwResult = RegSetValueEx ((HKEY) *phKey, KeyName, 0, REG_BINARY, (PBYTE) pBuffer, (wcslen(pBuffer)*2)+2)) != ERROR_SUCCESS) break;
free(uzBuffer);
free(pbBuffer);
return TRUE;
case CLSEKEY:
if (RegCloseKey ((HKEY) *phKey) != ERROR_SUCCESS) break;
free(uzBuffer);
free(pbBuffer);
return TRUE;
}
printf(“KeyName=‘%ws’ RegistryType=‘%d’.\n”, KeyName, RegistryType);
printf(“dwResult=‘%d’.\n”, dwResult);
free(uzBuffer);
free(pbBuffer);
return FALSE;
}

The code I use to call it is.
/* if fCheck is set to true there is a time limit for a reply. */
if(pDrvSubMessageRecv->fCheck == TRUE)
{
/* This works. */
printf(“fCheck is set.\n”);
pDrvMessageSend->DrvSubMessageSend.fCheck = TRUE;
pDrvMessageSend->DrvSubMessageSend.fAllow = TRUE;

/* Send back to the driver. */
hResult = FilterReplyMessage(pThreadContext->hPort, (PFILTER_REPLY_HEADER) pDrvMessageSend, sizeof(DRV_MESSAGE_SEND));
if(!SUCCEEDED(hResult))
{
if(StringCchPrintfA (ErrorMessage, MAXIMUM_ERROR_MESSAGE_SIZE, “FilterReplyMessage Error replying message. fCheck. hResult=‘0x%X’ GetLastError=‘%d’.\n”, hResult, GetLastError()) == S_OK) WriteToLog(ErrorMessage);
printf(“[9th].\n”); exit(0); break;
}
}
else
{
/* This freezes while installing visual studio 6. */
printf(“fCheck is not set.\n”);
pDrvMessageSend->DrvSubMessageSend.fAllow = TRUE;
fCreate = 1; fRead = 1; fWrite = 1; bExecute = 1; fAlways = 0;

/* If the connected application pid is this pid, allow everything. */
if(Registry(HKEY_LOCAL_MACHINE, L"Software\Program", &hKey, NULL, NULL, 0, OPENKEY) == TRUE)
{
if(Registry(HKEY_LOCAL_MACHINE, L"SafePid", &hKey, NULL, &dSafePid, 0, DGETKEY) == FALSE) break;
if(Registry(HKEY_LOCAL_MACHINE, L"Software\Program", &hKey, NULL, NULL, 0, CLSEKEY) == FALSE) break;
}

/* Send back to the driver. */
hResult = FilterReplyMessage(pThreadContext->hPort, (PFILTER_REPLY_HEADER) pDrvMessageSend, sizeof(DRV_MESSAGE_SEND));
if(!SUCCEEDED(hResult))
{
if(StringCchPrintfA (ErrorMessage, MAXIMUM_ERROR_MESSAGE_SIZE, “FilterReplyMessage Error replying message. hResult=‘0x%X’ GetLastError=‘%d’.\n”, hResult, GetLastError()) == S_OK) WriteToLog(ErrorMessage);
printf(“[7th].\n”); exit(0); break;
}

xxxxx@hotmail.co.uk wrote:

Could someone look over the following, it’s causing my program to freeze up and subsequently because the program is not responding windows itself.

It is difficult for me to imagine a worse interface for accessing the
registry. The interface you have chosen is so error-prone that I’m
surprised you even got this far. I have composed several paragraphs of
questions and suggestions regarding your choices and coding style, but
in the interest of brevity, I will limit myself to pointing out the problem:

case DGETKEY:
if (pBuffer != NULL) break;
if ((dwResult = RegQueryValueEx ((HKEY) *phKey, KeyName, 0, &dwRegType, (LPBYTE) &pbBuffer, &dwLocalLength)) != ERROR_SUCCESS) break;

You have an extra & there. It should be (LPBYTE)pbBuffer, not
(LPBYTE)&pbBuffer.


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

It was actually tried to remove as many possible errors as possible.

What would you suggest I did to improve it? I am still having the freeze problems and I could really do with the learning.

N.

xxxxx@hotmail.co.uk wrote:

It was actually tried to remove as many possible errors as possible.

You’ve got to be kidding. You pass in two keys, sometimes you use one,
sometimes the other. You pass two buffers to receive output data,
sometimes you use one, sometimes the other. Sometimes KeyName is a key
name, sometimes it is a value name. You’re allocating memory that you
never use. You read a DWORD value in, convert it to a string, then
convert it back to a DWORD. You have casts where they aren’t needed.
The whole thing is a mess.

When you create a registry key under HKLM\Software, you should use a
specific name for you (or your company), not just “Program”. Also, if
you plan for this to work without being elevated, you should use
HKEY_CURRENT_USER, not HKEY_LOCAL_MACHINE.

What would you suggest I did to improve it? I am still having the freeze problems and I could really do with the learning.

I’d throw the whole function out and use the ATL CRegKey class.

DWORD dSafePid = 0;

CRegKey key;
if( key.Open( HKEY_LOCAL_MACHINE, “Software\Program”,
KEY_ALL_ACCESS ) == ERROR_SUCCESS )
{
key.QueryDWORDValue( “SafePid”, &dSafePid );
}


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

Is there a C alternative? The whole program has been written in C and there is a lot to change to have it happy as a C++. Maybe I should relook at it all.

What are you trying to do? You could very easily do this in a Powershell
script or even C# and .Net Framework, easier than what have tried to do in
C.

Gary G. Little
H (952) 223-1349
C (952) 454-4629
xxxxx@comcast.net

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of
xxxxx@hotmail.co.uk
Sent: Wednesday, December 02, 2009 1:55 PM
To: Windows System Software Devs Interest List
Subject: RE:[ntdev] Struggling with some code.

Is there a C alternative? The whole program has been written in C and there
is a lot to change to have it happy as a C++. Maybe I should relook at it
all.


NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at
http://www.osronline.com/page.cfm?name=ListServer

__________ Information from ESET Smart Security, version of virus signature
database 4655 (20091202) __________

The message was checked by ESET Smart Security.

http://www.eset.com

__________ Information from ESET Smart Security, version of virus signature
database 4656 (20091202) __________

The message was checked by ESET Smart Security.

http://www.eset.com

xxxxx@hotmail.co.uk wrote:

Is there a C alternative? The whole program has been written in C and there is a lot to change to have it happy as a C++. Maybe I should relook at it all.

Maybe. I’m very surprised that anyone would undertake a significant
application in straight C today.

At the very least, get rid of the silly dword-to-string-to-dword
sequence, by replacing this:

if ((dwResult = RegQueryValueEx ((HKEY) *phKey, KeyName, 0,
&dwRegType, (LPBYTE) &pbBuffer, &dwLocalLength)) != ERROR_SUCCESS)
break;
if (StringCchPrintfA(uzBuffer, 11, “%d”, pbBuffer) != S_OK)
break;
*dwBuffer = atoi(uzBuffer);

with this:
if( RegQueryValueEx( *phKey, KeyName, 0, &dwRegType,
(LPBYTE)dwBuffer, &dwLocalLength ) != ERROR_SUCCESS )
break;


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

> At the very least, get rid of the silly dword-to-string-to-dword

sequence, by replacing this:

No, the mixing relations of the epoxy are not correct there. (SCNR.)

I took your advice and I did that a few posts ago.