Issue #41 - Bring method of determining IE version up to date

src/shared/Registry.cpp

 #include "Registry.h"
 #include <memory>
 
 using namespace AdblockPlus;
 
-Registry_Key::Registry_Key(HKEY parent, const std::wstring& key_name)

[sergei, 2014/10/21 11:18:33]

I think we use different naming convention, without underscore.

[Eric, 2015/01/04 23:06:03]

Done.

+RegistryKey::RegistryKey(HKEY parent, const std::wstring& key_name)
 {
   if (key_name.empty())
   {
     throw std::runtime_error("key_name may not be empty");
   }
   HRESULT hr = RegOpenKeyExW(parent, key_name.c_str(), 0, KEY_QUERY_VALUE, &key);
   if (hr != ERROR_SUCCESS || !key)
   {
     throw std::runtime_error("Failure in RegOpenKeyExW");
   }
 }
 
-Registry_Key::~Registry_Key()
+RegistryKey::~RegistryKey()
 {
   RegCloseKey(key);
 }
 
-std::wstring Registry_Key::value_wstring(const std::wstring& name) const
+std::wstring RegistryKey::value_wstring(const std::wstring& name) const
 {
   /*
    * Step one is to determine the presence of the value, along with its type and byte size.
    */
   DWORD type;
   DWORD size = 0;
   HRESULT hr = ::RegQueryValueExW(static_cast<HKEY>(key), name.c_str(), 0, &type, 0, &size);
   if (hr != ERROR_SUCCESS)
   {
     throw std::runtime_error("Failure in RegQueryValueEx to query name");
   }
   if (type != REG_SZ)
   {
     throw std::runtime_error("Value is not string type");
   }
 
   /*
    * Step two is to allocate a buffer for the string and query for its value.
    * Note that 'size' is the size in bytes, which we need for the system call,
    *   but that 'psize' is the size in words, which we need to manipulate the wchar_t array 'p'.
    */
+  size_t psize = (size + 1)/2;
   // Round the byte size up to the nearest multiple of two.
-  size = (size + 1) & ~ DWORD(1);
-  size_t psize = size >> 1;

[sergei, 2014/10/21 11:18:33]

It's not critical, only my opinion:
- I don't like using two variables psize and size below, using only one is
better.
- Despite here it looks correct, there is so plenty of undefined and
implementation defined behaviour with bit logic that if it's possible I would
like to avoid it.

[Eric, 2015/01/04 23:06:03]

Can't do it with just one variable without introducing redundant calls.
RegQueryValueExW  modifies its argument. If we don't save the length of 'p'
across that call, we would have to call 'strlen' again.
Rewritten to use integer arithmetic. No dependency on binary representation (not
that that's ever goes to change for IE).

+  size = 2 * psize;
   // We need to allocate a temporary buffer to receive the value, because there's no interface to write directly into the buffer of std::basic_string.
-  std::unique_ptr< wchar_t[] > p(new wchar_t[psize]);

[sergei, 2014/10/21 11:18:33]

no additional spaces

[Eric, 2015/01/04 23:06:03]

Done.

+  std::unique_ptr<wchar_t[]> p(new wchar_t[psize]);
   hr = RegQueryValueExW(key, name.c_str(), 0, 0, reinterpret_cast<BYTE*>(p.get()), &size);
   if (hr != ERROR_SUCCESS)
   {
     throw std::runtime_error("Failure in RegQueryValueExW to retrieve value");
   }
 
   /*
    * Step three is to construct a return string.
    *
    * There's the possibility of an extra terminating null character in the return value of the query.
    * If so, we have to decrement the length of the return value to eliminate it.
    * If it persists, it will interfere with later string operations such as concatenation.
    */
   if (p[psize - 1] == L'\0')
   {
     --psize;
   }
   return std::wstring(p.get(), psize);
 }