Can't get custom MFC CSettingsStore to work :-(

jwatte's picture

I want to implement my own CSettingsStore that stores data in a file in local application data, rather than the registry. (I note that the documentation claims this is a good idea from a security point of view, but I'm doing it for other reasons).

I overrode all the virtual functions in CSettingsStore and made them read/write an internal std::map>, and separately serialize that data structure to the file when dirty. The tests I've run on the class in isolation seem to all work fine.

However, when I use this class to save/restore docking pane and toolbar state and location, my docking panes end up being restored in a state where they are not visible, but the menu believes they are. If I disable my class, and use the registry, everything works, so there's something wrong with my class, but I can't figure out what it could be. Unfortunately, the documentation for CSettingsStore is a joke (along the lines of "CSettingsStore::Write() : this function writes data to the settings store") so I have no way of testing whether my subclass is properly implemented or not, except by trial and error.

If I were to design a class like this, I would make all of the Write() functions not virtual, and vector it all through a WriteInternal() function that takes a name, a pointer/size pair, and perhaps a data type indicator (as it integrates with the registry). However, from what I can tell, that's not what I'm supposed to do, so I overrode all the functions separately.

Even worse, btw: I notice that there is no documentation for how the memory returned by Read(..., BYTE **, ...) should be allocated. Some parts of the MFC library delete this data with delete[], and others call free() on that pointer. Through sheer coincidence, that doesn't crash the C and C++ heap implementations, but that's pretty iffy if you ask me. (And, again, the documentation doesn't go near trying to talk about what the expected behavior should be).

Here is my implementation, if someone wants to take a gander.

#pragma once
#include "afxsettingsstore.h"
 
class CLocalSettingsStore :
  public CSettingsStore
{
public:
  DECLARE_DYNCREATE(CLocalSettingsStore)
  CLocalSettingsStore();
  ~CLocalSettingsStore();
 
  HRESULT ReadFromFile();
  HRESULT WriteToFile();
  void Clear();
  void SetCurKey(wchar_t const *text);
 
	virtual BOOL CreateKey(LPCTSTR lpszPath);
	virtual BOOL Open(LPCTSTR lpszPath);
	virtual void Close();
 
	virtual BOOL DeleteValue(LPCTSTR lpszValue);
	virtual BOOL DeleteKey(LPCTSTR lpszPath, BOOL bAdmin = FALSE);
 
  //  The functions that actually need to be overridden are:
  //  DWORD
  //  string
  //  byte pointer
  //  This is not documented, but that's what the source says.
 
	virtual BOOL Write(LPCTSTR lpszValueName, DWORD dwVal);
	virtual BOOL Write(LPCTSTR lpszValueName, LPCTSTR lpszVal);
	virtual BOOL Write(LPCTSTR lpszValueName, LPBYTE pData, UINT nBytes);
 
	virtual BOOL Read(LPCTSTR lpszValueName, DWORD& dwValue);
	virtual BOOL Read(LPCTSTR lpszValueName, CString& strValue);
	virtual BOOL Read(LPCTSTR lpszValueName, BYTE** ppData, UINT* pcbData);
 
  static std::map<std::wstring, std::vector<char>> keys_;
  static bool dirty_;
  std::wstring curKey_;
};

#include "StdAfx.h"
#include "LocalSettingsStore.h"
#include <cctype>
 
#pragma warning(disable: 4996)
 
// todo: update this define, and remove the #undef on the line below
#define APPLICATION_NAME L"YourApplication"
#undef APPLICATION_NAME
 
#define SETTINGS_VERSION_NUMBER 0xE0F00001
#define SETTINGS_END_NUMBER 0x00009F70
#define MAX_VALUE_SIZE (128*1024)
#define MAX_VALUE_NAME_LENGTH 1024
 
 
IMPLEMENT_DYNCREATE(CLocalSettingsStore, CSettingsStore)
 
static int nWrites;
std::map<std::wstring, std::vector<char>> CLocalSettingsStore::keys_;
bool CLocalSettingsStore::dirty_;
 
static std::wstring Downcase(std::wstring str)
{
  std::wstring ret(str);
  std::transform(ret.begin(), ret.end(), ret.begin(), towlower);
  return ret;
}
 
CLocalSettingsStore::CLocalSettingsStore(void)
{
  //  Don't re-load keys from disk all the time!
  //  The registry reading/writing is really aggressive :-(
  if (keys_.size() == 0)
  {
    ReadFromFile();
  }
  dirty_ = false;
}
 
CLocalSettingsStore::~CLocalSettingsStore(void)
{
  if (dirty_)
  {
    ++nWrites;
    WriteToFile();
  }
}
 
 
HRESULT CLocalSettingsStore::ReadFromFile()
{
  wchar_t path[MAX_PATH + 1];
  HRESULT hr;
  std::wstring name;
  std::vector<char> data;
  if ((hr = ::SHGetFolderPath(0, CSIDL_FLAG_CREATE | CSIDL_LOCAL_APPDATA, 0, SHGFP_TYPE_CURRENT, path)) == S_OK)
  {
    std::wstring p(path);
    p += L"\\";
    p += APPLICATION_NAME;
    ::CreateDirectory(p.c_str(), 0);
    p += L"\\Settings.bin";
    FILE *f = _wfopen(p.c_str(), L"rb+");
    if (!f)
      goto error;
    unsigned int version = 0;
    if (1 != fread(&version, 4, 1, f))
      goto error;
    if (version != SETTINGS_VERSION_NUMBER)
      goto error;
    unsigned short keyCount = 0;
    if (1 != fread(&keyCount, 2, 1, f))
      goto error;
    for (unsigned int i = 0; i != keyCount; ++i)
    {
      unsigned short nameSize;
      if (1 != fread(&nameSize, 2, 1, f))
        goto error;
      //  limit the length of names
      if (nameSize < 1 || nameSize > MAX_VALUE_NAME_LENGTH)
        goto error;
      unsigned short usePrev;
      if (1 != fread(&usePrev, 2, 1, f))
        goto error;
      if (usePrev >= nameSize)
        goto error;
      name.resize(nameSize);
      if (nameSize != fread(&name[0] + usePrev, sizeof(wchar_t), nameSize - usePrev, f))
        goto error;
      unsigned int keySize;
      if (1 != fread(&keySize, 4, 1, f))
        goto error;
      //  limit size to something sane
      if (keySize > MAX_VALUE_SIZE)
        goto error;
      data.resize(keySize);
      if (keySize != 0 && keySize != fread(&data[0], 1, keySize, f))
        goto error;
      keys_[Downcase(name)] = data;
      std::wstringstream wss;
    }
    if (1 != fread(&version, 4, 1, f))
      goto error;
    if (version != SETTINGS_END_NUMBER)
      goto error;
    fclose(f);
    return S_OK;
 
error:
    ::OutputDebugString(L"LocalSettingsStore: Corrupted settings found. Reverting to defaults.\r\n");
    if (f)
      fclose(f);
    //  use no settings if the read didn't work
    Clear();
    return E_FAIL;
  }
  else
  {
    return hr;
  }
}
 
size_t CommonPrefixLength(std::wstring const &a, std::wstring const &b)
{
  std::wstring::const_iterator ap = a.begin(), end = a.end();
  std::wstring::const_iterator bp = b.begin();
  size_t len = 0;
  while (ap != end)
  {
    if (*ap != *bp)
      return len;
    ++len;
    ++ap;
    ++bp;
  }
  return len;
}
 
HRESULT CLocalSettingsStore::WriteToFile()
{
  wchar_t path[MAX_PATH + 1];
  HRESULT hr;
  std::map<std::wstring, std::vector<char>>::iterator ptr, end;
  if ((hr = ::SHGetFolderPath(0, CSIDL_FLAG_CREATE | CSIDL_LOCAL_APPDATA, 0, SHGFP_TYPE_CURRENT, path)) == S_OK)
  {
    std::wstring prev = L"";
    std::wstring p(path);
    p += APPLICATION_NAME;
    ::CreateDirectory(p.c_str(), 0);
    p += L"\\Settings.bin";
    std::wstring q(p);
    q += L".tmp";
    FILE *f = _wfopen(q.c_str(), L"wb+");
    if (!f)
      goto error;
    unsigned int version = SETTINGS_VERSION_NUMBER;
    if (1 != fwrite(&version, 4, 1, f))
      goto error;
    unsigned short keyCount = (unsigned int)keys_.size();
    if (1 != fwrite(&keyCount, 2, 1, f))
      goto error;
    ptr = keys_.begin(), end = keys_.end();
    for (unsigned int i = 0; i != keyCount; ++i)
    {
      if (ptr == end)
        goto error;
      unsigned short nameSize = (*ptr).first.length();
      //  limit the length of names
      if (nameSize < 1 || nameSize > MAX_VALUE_NAME_LENGTH)
        goto error;
      unsigned short usePrev = CommonPrefixLength(prev, (*ptr).first);
      if (1 != fwrite(&nameSize, 2, 1, f))
        goto error;
      if (1 != fwrite(&usePrev, 2, 1, f))
        goto error;
      if ((nameSize - usePrev) != fwrite(&(*ptr).first[0] + usePrev, sizeof(wchar_t), nameSize - usePrev, f))
        goto error;
      unsigned int keySize = (*ptr).second.size();
      //  limit size to something sane
      if (keySize > 128*1024)
        goto error;
      if (1 != fwrite(&keySize, 4, 1, f))
        goto error;
      if (keySize != 0 && keySize != fwrite(&(*ptr).second[0], 1, keySize, f))
        goto error;
      ++ptr;
    }
    version = SETTINGS_END_NUMBER;
    if (1 != fwrite(&version, 4, 1, f))
      goto error;
    fclose(f);
    f = NULL;
    _wremove(p.c_str());
    /* A crash right here would cause the settings to be lost. Oh, well. */
    if (_wrename(q.c_str(), p.c_str()) != 0)
      goto error;
    dirty_ = false;
    return S_OK;
error:
    ::OutputDebugString(L"LocalSettingsStore: Error writing settings. Old settings are preserved.\r\n");
    if (f)
    {
      fclose(f);
      _wremove(q.c_str());
    }
    return E_FAIL;
  }
  else
  {
    return hr;
  }
}
 
void CLocalSettingsStore::Clear()
{
  std::map<std::wstring, std::vector<char>>().swap(keys_);
  dirty_ = true;
}
 
 
void CLocalSettingsStore::SetCurKey(wchar_t const *key)
{
  curKey_ = Downcase(key);
  if (!curKey_.empty() && *curKey_.rbegin() != L'\\')
    curKey_.append(L"\\");
}
 
BOOL CLocalSettingsStore::CreateKey(LPCTSTR lpszPath)
{
  std::wstring key(lpszPath);
  if (key.length() == 0)
    return FALSE;
  SetCurKey(lpszPath);
  keys_[curKey_] = std::vector<char>();
  dirty_ = true;
  return TRUE;
}
 
BOOL CLocalSettingsStore::Open(LPCTSTR lpszPath)
{
  //  the key is represented by a 0-length value of the key name with a terminating slash
  std::wstring key(lpszPath);
  if (key.length() == 0)
    return FALSE;
  if (*key.rbegin() != '\\')
    key.append(L"\\");
  std::map<std::wstring, std::vector<char>>::iterator pos = keys_.find(Downcase(key));
  if (pos == keys_.end())
    return FALSE;
  SetCurKey(key.c_str());
  return TRUE;
}
 
void CLocalSettingsStore::Close()
{
  SetCurKey(L"");
}
 
 
BOOL CLocalSettingsStore::DeleteValue(LPCTSTR lpszValue)
{
  std::wstring name(lpszValue);
  if (name.length() == 0 || *name.rbegin() == '\\')
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::map<std::wstring, std::vector<char>>::iterator ptr = keys_.find(Downcase(key));
  if (ptr == keys_.end())
    return FALSE;
  keys_.erase(ptr);
  dirty_ = true;
  return TRUE;
}
 
BOOL CLocalSettingsStore::DeleteKey(LPCTSTR lpszPath, BOOL bAdmin)
{
  if (curKey_ == lpszPath)
    return FALSE;
  std::wstring key(lpszPath);
  if (key.length() == 0)
    return FALSE;
  if (*key.rbegin() != '\\')
    key.append(L"\\");
  std::map<std::wstring, std::vector<char>>::iterator ptr = keys_.find(Downcase(key));
  if (ptr == keys_.end())
    return FALSE;
  std::map<std::wstring, std::vector<char>>::iterator end = keys_.end();
  //  erase everything that starts with whatever this key path is
  while (ptr != end)
  {
    if ((*ptr).first.length() < key.length())
      break;
    std::wstring name((*ptr).first);
    name.resize(key.length());
    if (name != key)
      break;
    keys_.erase(ptr++);
  }
  dirty_ = true;
  return TRUE;
}
 
 
BOOL CLocalSettingsStore::Write(LPCTSTR lpszValueName, DWORD dwVal)
{
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\' || name.length() > MAX_VALUE_NAME_LENGTH)
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::vector<char> v;
  v.resize(sizeof(dwVal));
  memcpy(&v[0], &dwVal, v.size());
  keys_[Downcase(key)] = v;
  dirty_ = true;
  return TRUE;
}
 
BOOL CLocalSettingsStore::Write(LPCTSTR lpszValueName, LPCTSTR lpszVal)
{
  if (lpszVal == NULL)
    lpszVal = L"";
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\' || name.length() > MAX_VALUE_NAME_LENGTH)
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::vector<char> v;
  v.resize((wcslen(lpszVal) + 1) * sizeof(wchar_t));
  if (v.size() > MAX_VALUE_SIZE)
    return FALSE;
  memcpy(&v[0], lpszVal, v.size());
  keys_[Downcase(key)] = v;
  dirty_ = true;
  return TRUE;
}
 
BOOL CLocalSettingsStore::Write(LPCTSTR lpszValueName, LPBYTE pData, UINT nBytes)
{
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\' || name.length() > MAX_VALUE_NAME_LENGTH)
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::vector<char> v;
  v.resize(nBytes);
  if (v.size() > MAX_VALUE_SIZE)
    return FALSE;
  if (v.size() > 0)
    memcpy(&v[0], pData, v.size());
  keys_[Downcase(key)] = v;
  dirty_ = true;
  return TRUE;
}
 
 
BOOL CLocalSettingsStore::Read(LPCTSTR lpszValueName, DWORD& dwValue)
{
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\')
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::map<std::wstring, std::vector<char>>::iterator ptr = keys_.find(Downcase(key));
  if (ptr == keys_.end())
    return FALSE;
  if ((*ptr).second.size() != sizeof(dwValue))
    return FALSE;
  memcpy(&dwValue, &(*ptr).second[0], (*ptr).second.size());
  return TRUE;
}
 
BOOL CLocalSettingsStore::Read(LPCTSTR lpszValueName, CString& strValue)
{
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\')
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::map<std::wstring, std::vector<char>>::iterator ptr = keys_.find(Downcase(key));
  if (ptr == keys_.end())
    return FALSE;
  if ((*ptr).second.size() == 0)
    return FALSE;
  if ((*ptr).second.size() % sizeof(wchar_t) != 0)
    return FALSE;
  wchar_t const *str = (wchar_t const *)&(static_cast<char &>((*ptr).second[0]));
  size_t len = (*ptr).second.size() / sizeof(wchar_t) - 1;
  //  must be zero terminated
  if (str[len] != 0)
    return FALSE;
  strValue.SetString(str);
  return TRUE;
}
 
 
BOOL CLocalSettingsStore::Read(LPCTSTR lpszValueName, BYTE** ppData, UINT* pcbData)
{
  *ppData = 0;
  std::wstring name(lpszValueName);
  if (name.length() == 0 || *name.rbegin() == '\\')
    return FALSE;
  std::wstring key(curKey_);
  key += name;
  std::map<std::wstring, std::vector<char>>::iterator ptr = keys_.find(Downcase(key));
  if (ptr == keys_.end())
    return FALSE;
  *pcbData = (*ptr).second.size();
  //  Curiously, the BYTE* returned should be allocated with new[].
  //  This is not documented, but is how the library calls it.
  *ppData = new BYTE[*pcbData];
  memcpy(*ppData, &(*ptr).second[0], *pcbData);
  return TRUE;
}

I apply it in the application at the top of InitInstance() as follows:

BOOL CMyApp::InitInstance()
{
  CSettingsStoreSP::SetRuntimeClass(RUNTIME_CLASS(CLocalSettingsStore));
...

AttachmentSize
LocalSettingsStore.zip4.64 KB