1

I have to add a list of sub keys recursively to an item in a Tree control. I am using InsertItem as shown below. I am attaching the path of this element so that i can retrieve when clicked on the tree control.I am able to add the value but not able to retrieve it.

void CMyDlg::FillTreeWithRegistryKeysEx(CString sPath, HTREEITEM hItem)
{
    CString sRegKey = sPath.Left(sPath.Find(_T("\\")));
    sPath = sPath.Mid((sPath.Find(_T("\\")) + 1));
    sPath = CleanRegistryKey(sPath);

    int nKeyCount;
    CString  sSubKey = _T("");
    HKEY handle = GetHkey(sRegKey);
    HTREEITEM hReItem = NULL;
    HKEY phkey;

    std::vector<CString> sSubFolders;

    if (RegOpenKeyEx(handle, sPath, 0, KEY_ALL_ACCESS, &phkey) == ERROR_SUCCESS)
        sSubFolders = EnumRegistryKey(phkey);

    nKeyCount = sSubFolders.size();

    for (int nIndex = 0; nIndex < nKeyCount; nIndex++)
    {
        sSubKey = sPath + _T("\\") +sSubFolders.at(nIndex);
        hReItem = m_cTreeCtrl.InsertItem(TVIF_TEXT | TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_PARAM | TVIF_STATE, sSubFolders.at(nIndex), 
                                        icoClosedFolder, icoOpenFolder, 0, 0, (LPARAM)(LPCTSTR)sSubKey, hItem, TVI_LAST);

        FillTreeWithRegistryKeys(handle, sSubKey, hReItem);
    }
    RegCloseKey(phkey);
}


While retrieving the string is always blank.

void CMyDlg::OnTvnSelchangedTree(NMHDR *pNMHDR, LRESULT *pResult)
{
    LPNMTREEVIEW pNMTreeView = reinterpret_cast<LPNMTREEVIEW>(pNMHDR);

    LPARAM lp = pNMTreeView->itemNew.lParam;
    CString sKey = (LPCTSTR)lp;
}


what is going wrong? 
Sandeep Kumar
  • 85
  • 1
  • 3
  • 10
  • Is the conversion proper ? I doubt this could be the problem LPARAM lp = pNMTreeVie>itemNew.lParam; CString sKey = (LPCTSTR)lp; – Sandeep Kumar Sep 01 '16 at 06:58

2 Answers2

2

The application-defined LPARAM passed to InsertItem is a string object that goes out of scope once the function completes not to mention it is replaced with each loop iteration with a new string. I don't think that is going to work.

The TreeView control however allocates its own copy of the text, so you are safe to fill in the pszText member with local buffers as it appears you are doing.

byteptr
  • 1,275
  • 11
  • 15
  • I am using sSubKey as the key string and i also tried using it as a global variable but it dosent work. – Sandeep Kumar Sep 01 '16 at 06:51
  • Its also possible "sSubKey = sPath + _T("\\") +sSubFolders.at(nIndex);" with each iteration of the loop is being freed. Troubleshoot that InsertItem() can actually add a hardcoded string rather than "(LPARAM)(LPCTSTR)sSubKey". If a hardcoded string works, I think your sSubKey is getting freed. – byteptr Sep 01 '16 at 07:03
  • No it dosen't. I am able to get the LPARAM value while retrieving (the same value that i am inserting) but not the string associated – Sandeep Kumar Sep 01 '16 at 07:15
  • Sorry, I thought your LPARAM wasn't coming through. What function are you pulling out the string (pszText member), since you are testing your LPARAM in CMyDlg::OnTvnSelchangedTree()? – byteptr Sep 01 '16 at 07:22
  • If you want to retrieve the text, you need to use the GetItemText() method. – byteptr Sep 01 '16 at 08:10
  • 2
    You said "I am able to get the LPARAM value while retrieving (the same value that i am inserting) but not the string associated". The pointer you added as the LPARAM is being properly stored by the control, but you are witnessing that the contents at that memory location have been freed. You need to assign each item its own copy of the string, such as "new CString(sSubKey)" and then handle the TVN_DELETEITEM notification so you know when you can free these objects. – byteptr Sep 01 '16 at 08:31
  • I tried using new CString(sSubKey) and now i receive garbage in the string. I am working on Unicode – Sandeep Kumar Sep 01 '16 at 08:52
  • 1
    @SandeepKumar: The core issue is, that you have been assigning a pointer to an object with automatic storage duration. If you need to control an object's life time (and you do in your sample), you'll have to go with dynamic storage duration. This answer addresses your issue in full. If you follow the advice (assigning `new CString(sSubKey)` to `lParam`), and are still having issues, you should open a new question. As far as this question is concerned, it has been answered in full. – IInspectable Sep 01 '16 at 09:46
  • It works with CString(sSubKey) and while receiving CString *sKey = (CString *)lp; – Sandeep Kumar Sep 01 '16 at 09:58
  • Just use `m_cTreeCtrl.InsertItem(L"text");` and `string = m_cTreeCtrl.GetItemText(hItem);` You have been asking the same question over the last 2 weeks. Why do you insist on using `LPARAM` to store pointer to temporary string? – Barmak Shemirani Sep 01 '16 at 10:01
  • I have to replicate the registry tree structure in my application, I need to know where the user has clicked and based on which i need to do an operation. m_cTreeCtrl.InsertItem(L"text"); and string = m_cTreeCtrl.GetItemText(hItem); will get me the selected string but i need the path of it which i have to store while inserting. Probably you misunderstood the requirement . Please go through my snippet again – Sandeep Kumar Sep 02 '16 at 11:10
  • The path is simply the concatenation of the selected node's text with its parent node's text, and its parent node text, and so on, separating them with slashes, until you reach the base node. You can use a recursive function to build up that path string dynamically (just like you use a recursive function to fill the tree, just in reverse). You don't need to store the actual path strings in each node's `lParam`, that is just a waste of memory. – Remy Lebeau Sep 03 '16 at 05:57
  • @IInspectable : can you please suggest when to free the CString pointer i used in the snippet CString *sKey = (CString *)lp after taking the data from the lparam – Sandeep Kumar Jan 03 '17 at 14:19
  • @SandeepKumar: That pointer doesn't own the pointed-to resource. It only serves as a handle to the information, and never needs to be freed. However, that part of the code isn't even related to your issue: Storing pointers to objects that are no longer valid. – IInspectable Jan 03 '17 at 14:26
  • @IInspectable : I am inserting an item to the tree control in OnInitdialog() as below CString *stestString = new CString(_T("XXX")); HTREEITEM hregHkCUItem = m_cTreeCtrl.InsertItem((LPARAM)(LPCTSTR)stestString); I am doing the following code in event whenever tree item is changed, i am able to get the data into sTempKey in the below snippet. I can free the sTempKey in the following API itself.....continued in the next comment .... – Sandeep Kumar Jan 04 '17 at 08:30
  • @IInspectable:LPNMTREEVIEW pNMTreeView = reinterpret_cast(pNMHDR); LPARAM lp = pNMTreeView->itemNew.lParam; CString *sTempKey = (CString *)lp; Now my question is, Do i have to free the stestString that i created in my OnInitDialog() ?? If yes when should i do it ? I feel it should be freed as it can be dangling if i dont. please suggest Thank you – Sandeep Kumar Jan 04 '17 at 08:31
  • @SandeepKumar: You already have a dangling pointer, since you never store the result of `new CString( ... )` anywhere. You are storing the return value of [operator PCXSTR()](https://msdn.microsoft.com/en-us/library/sddk80xf.aspx#csimplestringt__operator_pcxstr), and then attempt to retrieve it by casting it (`(CString*)lp`). This is a bug. And since you keep struggling with basic pointer usage, you may want to pick up a book from [The Definitive C++ Book Guide and List](http://stackoverflow.com/q/388242/1889329). – IInspectable Jan 05 '17 at 11:04
1

As pointed out in previous answer, you are using LPARAM to point to a string which is destroyed when the function exits. The solution is store the strings in permanent storage. There are many ways to do this. One option is to use CStringArray member data:

class CMyDlg : public CDialog
{
    CStringArray m_strings;
    ...
};
  • Store the registry keys in m_strings.
  • Add the tree item with InsertItem, this returns HTREEITEM.
  • Use tree.SetItemData to match the tree item with an index in m_strings
  • Use tree.GetItemData to retrieve that index

Example:

BOOL CMyDlg::OnInitDialog()
{
    CDialog::OnInitDialog();
    ...
    m_strings.RemoveAll();
    addKey(L"SOFTWARE\\Microsoft\\Internet Explorer", tree.GetRootItem());
    return TRUE;
}

void CMyDlg::addKey(CString path, HTREEITEM parent)
{
    CRegKey rg;
    if (ERROR_SUCCESS != rg.Open(HKEY_CURRENT_USER, path))
        return;
    for (int i = 0;; i++)
    {
        wchar_t buf[300];
        DWORD bufsize = 300;
        if (rg.EnumKey(i, buf, &bufsize) != ERROR_SUCCESS) break;
        HTREEITEM hitem = tree.InsertItem(buf, parent);
        CString subkey = path + L"\\" + buf;
        m_strings.Add(subkey);
        tree.SetItemData(hitem, m_strings.GetCount() - 1);
        addKey(subkey, hitem);
    }
}

void CMyDlg::OnTvnSelchangedTree(NMHDR *pNMHDR, LRESULT *pResult)
{
    HTREEITEM hitem = tree.GetSelectedItem();
    if (!hitem) return;
    int i = tree.GetItemData(hitem);
    if (i >= 0 && i < m_strings.GetCount())
        SetWindowText(m_strings[i]);
}
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77
  • Thanks for the help, but the problem here is i don't want to exactly replicate the registry strings. Lets say my registry structure is abc->def->ghi (def is child of abc and ghi is child of def) but ets say i wanted to show it to user as first3letters->next3letters->last3letters. Now when i click on the tree control. i can only get (first3letters->next3letters->last3letters) but i need (abc->def->ghi) based on this info i have to fill a listctrl. So i I need to store the (abc->def->ghi) which i have to retrieve back. – Sandeep Kumar Sep 06 '16 at 04:56