Win32 registry functions failing

[caut_baia]

Hi everyone.I have a strange problem.I've written a class which handles registry keys and values of which there are two functions

template<> unsigned long RegistryEntry::GetValue <unsigned long> (std::string Name)  {    
      unsigned long Value;
      unsigned long DataSize;
                  
      assert (RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)&Value,
                               &DataSize)==ERROR_SUCCESS);
      return Value;
    }

template <> char* RegistryEntry::GetValue <char*> (std::string Name)  {
      char Value[1024];
      memset (Value,0,sizeof(Value)/sizeof(char));
      unsigned long DataSize;
                  
      assert (RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)Value,
                               &DataSize)==ERROR_SUCCESS);
      return Value;
    }

When called subsequently the second one always crashes unless 'i give it a break'

RegistryEntry Reg(HKEY_CURRENT_USER,"Booth"); 
std::cout << Reg.GetValue <unsiged long> ("one");
Sleep(1);  //without this the following function fails
std::cout << Reg.GetValue <char*> ("two");

I don't know much about pipeline stalls or whatever but i think there shouldn't be a problem with WinApi functions which are written by professionals.What could be the problem ?

[nezachem]

The fact that the second function works at all is an unfortunate coincidence. It returns a pointer to a local variable. You must allocate Value dynamically.

[caut_baia]

It doesn't work

[Ancient Dragon]

nezachem is correct -- you have to allocate memory for the value using new. If that didn't work, then post your new code.

You must set DataSize = number of bytes in Value before calling RegQueryValueEx().

assert() only works when you compile the program for debug. It does nothing at all when the program is compiled without debug. So if the release mode program has an error you would never know about it except that the program might crash and burn. It would be better to call GetLastError() then FormatMessage() and finally print the message returned by FormatMessage(). You will get consistent results in both debug and release mode builds.

[caut_baia]

I don't think it should be a problem since the return variable remains on the stack even after the automatic storage variables get destroyed .. from what i know.It shouldn't work at all if it got destroyed but it does.I did as he said even made it a member variable and it still doesn't work unless i sneak that Sleep call between them.What should GetLastError tell me ?:) "Give it a break?".Anyway i'll try as i wanted to try earlier but this doesn't look like a common problem.And thanks

[Ancient Dragon]

c++ standards state that you can not do that. The variable does NOT remain on the stack when the function returns because the state of the stack changes. That's the same as saying you can use a pointer after deleting it because the memory is still there. Well, it is, and it isn't.

>>What should GetLastError tell me
The error number (click here for details)

>>Anyway i'll try as i wanted to try earlier but this doesn't look like a common problem.And thanks
Ok, your funeral, not ours.

[caut_baia]

Well FormatMessage says "The specified procedure could not be found"

[Ancient Dragon]

Post all the code so that we can test it. It must be referring to some other error because I know that RegQueryValueEx() exists.

[caut_baia]

You're right about the pointer .. i gave it some thought.The memory gets deleted but the pointer which points to nothing..

[caut_baia]

The call to RegQueryValueEx invalidates the HKEY parameter for a period of time.If i make a call to Sleep or reassign the HKEY both functions work though i don't know why the handle gets invalid after querying a value.Very odd ..

[Ancient Dragon]

My guess is that you are calling RegCloseKey() between those two function calls. Here's a non-template version that works

#include <Windows.h>
#include <iostream>
#include <string>
using std::cin;
using std::cout;
using std::string;

BOOL ShowError(long dwError)
{
    char buf[255]= {0};
    if( dwError != 0)
    {
    FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM,0, (DWORD)dwError,0,buf,sizeof(buf),0);
    cout << buf << '\n';
    return TRUE;
    }
    return FALSE;

}

unsigned long GetValue (HKEY Key,std::string Name,unsigned long& Value)  {    
      unsigned long DataSize;
      DataSize = sizeof(long);                  
      ShowError(RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)&Value,
                               &DataSize));

      return Value;
    }

char* GetValue  (HKEY Key,std::string Name, char Value[1024])  {
      //memset (Value,0,sizeof(Value)/sizeof(char));
      Value[0] = 0;
      unsigned long DataSize = 1024;
                  
      ShowError(RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)Value,
                               &DataSize));

      return Value;
    }

int main()
{
    HKEY hKey = 0;
    char strValue[255] = {0};
    unsigned long dwValue = 0;
    if( ShowError(RegOpenKey(HKEY_LOCAL_MACHINE,"SOFTWARE\\Microsoft\\Active Setup",&hKey)) )
    {
        return 1;
    }
    GetValue(hKey,"JITSetupPage",strValue);
    GetValue(hKey,"DisableRepair",dwValue);

    RegCloseKey(hKey);
    cout << strValue << '\n';
    cout << dwValue << '\n';
}

[caut_baia]

#ifndef REG_H
#define REG_H

#include <windows>
#include <string>
#include <vector>
#include <assert>
#include "err.h"

class RegistryEntry  {
   std::string MainKeyName;
   HKEY Key;
   HKEY Subkey;
   std::vector<HKEY> Subkeys;

   char szValue[1024];
   unsigned long dwValue;

   void DeleteKey (HKEY);
public:
   RegistryEntry ();
   RegistryEntry (const HKEY&,std::string);
   ~RegistryEntry ();

   bool AddSubkey (std::string);
   
   HKEY OpenKey (const HKEY&,std::string);
   void Delete ();

   HKEY GetRootKey () const;
   std::vector<HKEY> AllSubkeys ();
   HKEY SearchKey (const HKEY&) const;
   operator HKEY () const;   

   
   bool SetValue (std::string,unsigned long); //REG_DWORD
   bool SetValue (std::string,std::string);               //REG_SZ

   template <typename T> T GetValue (std::string Name);   

 };                 

template<> unsigned long RegistryEntry::GetValue <unsigned long> (std::string Name)  {
   unsigned long DataSize;
                  
   if (!RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)&dwValue,
                         &DataSize)==ERROR_SUCCESS) ErrorMessage();
   return dwValue;
 }

template <> char* RegistryEntry::GetValue <char*> (std::string Name)  {
   memset (szValue,0,sizeof(szValue));
   unsigned long DataSize;
                  
   if (!RegQueryValueEx (Key,Name.c_str(),0,NULL,(unsigned char*)szValue,
                         &DataSize)==ERROR_SUCCESS) ErrorMessage();      
   return szValue;
 }


#endif

The cpp file

#include "reg.h"

//------------------------------------------------------------------------------------
RegistryEntry::RegistryEntry () {}
//------------------------------------------------------------------------------------
RegistryEntry::~RegistryEntry () {}
//------------------------------------------------------------------------------------
RegistryEntry::RegistryEntry (const HKEY& Key_,std::string Name_) :
   MainKeyName(Name_)  {
      RegCreateKeyEx (Key_,MainKeyName.c_str(),0,NULL,REG_OPTION_NON_VOLATILE,
                      KEY_ALL_ACCESS,NULL,&Key,NULL);
 };
//------------------------------------------------------------------------------------
HKEY RegistryEntry::GetRootKey () const {
   return Key;
 }
//------------------------------------------------------------------------------------
bool RegistryEntry::AddSubkey (std::string Name)  {   
   return RegCreateKeyEx(Key,Name.c_str(),0,NULL,REG_OPTION_NON_VOLATILE,
                         KEY_ALL_ACCESS,NULL,&Subkey,NULL)==ERROR_SUCCESS;   
 }
//------------------------------------------------------------------------------------
std::vector<HKEY> RegistryEntry::AllSubkeys ()  {   
   int Indx=0;   
   char Buff[1024];
   unsigned long BuffSize;
   HKEY Subkey;

   while (RegEnumKey (Key,Indx,Buff,BuffSize)!=ERROR_NO_MORE_ITEMS)  {
      RegOpenKeyEx (Key,Buff,0,KEY_ALL_ACCESS,&Subkey);
      Subkeys.push_back (Subkey);
      Indx++;
    }
   return Subkeys;
 }
//------------------------------------------------------------------------------------
void RegistryEntry::DeleteKey (HKEY Key_)  {   
   HKEY tmpKey;
   static const HKEY Root=Key_;      
           
   char Buff[1024];
   memset (Buff,0,sizeof(Buff));
   unsigned long BuffSize;
  
   if (RegEnumKeyEx (Key_,0,Buff,&BuffSize,0,0,0,0)!=ERROR_NO_MORE_ITEMS)  {   
      if (RegOpenKeyEx (Key_,Buff,0,KEY_ALL_ACCESS,&tmpKey)==ERROR_SUCCESS)  {
         DeleteKey (tmpKey);
       }
    }
    
   else  {
      RegDeleteKey (Key_,Buff);         
      DeleteKey (Root);
    }               
 }
//------------------------------------------------------------------------------------
HKEY RegistryEntry::OpenKey (const HKEY& UpperKey,std::string KeyName)  {   
   RegOpenKeyEx (UpperKey,KeyName.c_str(),0,KEY_ALL_ACCESS,&Key);
   return Key;
 }
//------------------------------------------------------------------------------------
void RegistryEntry::Delete ()  {
   DeleteKey (Key);
 }
//------------------------------------------------------------------------------------
RegistryEntry::operator HKEY () const {
   return Key;
 }
//------------------------------------------------------------------------------------
bool RegistryEntry::SetValue (std::string Name,unsigned long Value)  {                                        
   return RegSetValueEx (Key,Name.c_str(),0,REG_DWORD,
                        (unsigned char*) &Value,sizeof(Value));                        
 }
//------------------------------------------------------------------------------------ 
bool RegistryEntry::SetValue (std::string Name,std::string Value)  {
   const char* Val=Value.c_str();
   return RegSetValueEx (Key,Name.c_str(),0,REG_SZ,
                        (unsigned char*) Val,Value.length());                        
 }

The test file

#include <iostream>
#include "reg.h"

int main ()  {
   RegistryEntry Reg(HKEY_CURRENT_USER,"Proto");
   unsigned long x=11;
   std::string v="Qweweqeqweqwe";
   
   Reg.SetValue (std::string("long"),x);
   Reg.SetValue (std::string("char"),v);
   
//   std::cout << Reg.GetValue <unsigned long> (std::string("long")); 
//   std::cout << Reg.GetValue <char*> (std::string("char")); //does not work

   std::cout << RegistryEntry (HKEY_CURRENT_USER,"Proto").GetValue <unsigned long> (std::string("long")); //works

   std::cout << RegistryEntry (HKEY_CURRENT_USER,"Proto").GetValue <char*> (std::string("char"));
 }

[Ancient Dragon]

There are several problems with the code you posted.

1. reg.h line 1. There is no such file as <windows> -- Microsoft never removed the extension for c++ programs.

2. reg.cpp, RegistryEntry::AllSubkeys. Variable BuffSize is used before it has been initialized. initialize it to sizeof(Buf)

3. Move the implementation code for GetValue() from reg.h to reg.cpp because it causes duplicate declarations in every *.cpp file that contains reg.h.

[caut_baia]

Are these the warnings reported by VC++? I use borland 5.5 at the command line and i don't get them.I moved the functions as you said but that doesn't fix the problem and i don't see how it could.The class is not finished and i still have to work on some other code since this should be just an interface for storing some values so pardon my ugly style.

[Ancient Dragon]

Your program worked ok for me after making the changes I suggested.

>> I use borland 5.5 at the command line and i don't get them
Maybe you need to increase the warning levels issued by your compiler.

[caut_baia]

I've read the documentation and the default reports most of the errors.However i think i should go for visual c++ soon it's just that i like working at the command prompt with makefiles.Have you reached any idea of what i should do to avoid that Sleep call between the functions ?In any case thanks for your suggestions.

[Ancient Dragon]

Did you make the changes I said you needed to make? Your program ran ok for me without any Sleep() calls.

Do yourself a favor, save a lot of time, and get vc++ 2010 express so that you can easily debug your programs. You need to start coding in the 21st century.