Author Topic: Config save bug query  (Read 3140 times)

Offline AndrewCot

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 678
Config save bug query
« on: January 14, 2022, 04:05:25 am »
Sorry for yet another huge post, but it gets a little bit complex and I have included a potential untested solution.

I am working on getting my code for ticket 1114 ready for a patch to be submitted as allot of changes have occurred since August 2021.

I am ironing out some config bugs and want to check if my assumptions and findings are right or wrong. If they are wrong please point me on where I went wrong so I can get to the final solution faster.

I have found the following:
1) In the void CompilerFactory::SaveSettings() calls the following:
Code
void CompilerFactory::SaveSettings()
{
    // clear old keys before saving
    Manager::Get()->GetConfigManager(_T("compiler"))->DeleteSubPath(_T("/sets"));
    Manager::Get()->GetConfigManager(_T("compiler"))->DeleteSubPath(_T("/user_sets"));

    for (size_t i = 0; i < Compilers.GetCount(); ++i)
    {
        wxString baseKey = Compilers[i]->GetParentID().IsEmpty() ? _T("/sets") : _T("/user_sets");
        Compilers[i]->SaveSettings(baseKey);

   Which deletes all of the compilers.. Ouch
   And then at first glance of the code saves the settings, BUT the Compiler::SaveSettings(const wxString& baseKey) code only saves the changes since the call to the Compiler::MirrorCurrentSettings() that occurs on loading.... another Ouch but even more worring

Snippet of code from  Compiler::SaveSettings(const wxString& baseKey) :
Code
    if (m_Mirror.CompilerOptions_ != m_CompilerOptions)
    {
        wxString key = GetStringFromArray(m_CompilerOptions);
        cfg->Write(tmp + _T("/compiler_options"), key, false);
    }

2) The Compiler::SaveSettings(...) does not flush the file to disk at the end. Another minor change.

My query is:
Does anyone know why the CompilerFactory::SaveSettings() removed the keys and then the Compiler::SaveSetting() only saves the changes, so that if this code is executed then you could end up with a bad config file.  This is occurring with me for the mods I am making.


Potential Conclusion:
This could be what is causing the corrupted (aka bad) config files that are reported a few times are year as it depends on the code path as to if the problem outlined above occurs.


Possible Untested Solution:
a) Change the CompilerFactory::SaveSettings() to be:
Code
void CompilerFactory::SaveSettings()
{
    for (size_t i = 0; i < Compilers.GetCount(); ++i)
    {
        if (Compilers[i]->IsValid())
        {
            // Only save valid compiler configuration settings
            wxString baseKey = Compilers[i]->GetParentID().IsEmpty() ? _T("/sets") : _T("/user_sets");
            Compilers[i]->SaveSettings(baseKey);

            CodeBlocksEvent event(cbEVT_COMPILER_SETTINGS_CHANGED);
            event.SetString(Compilers[i]->GetID());
            event.SetInt(static_cast<int>(i));
            event.SetClientData(static_cast<void*>(Compilers[i]));
            Manager::Get()->ProcessEvent(event);
        }
    }
}
     The delete keys are moved into the SaveSettings(...) as it already deleted the old style keys
     This way only valid compiler data is saved instead of partial data for invalid compilers
c) Update the Compiler::SaveSettings(const wxString& baseKey) to do the following:
    1) Add the DeleteSubPath and the first comment line (the rest are exisiting lines)
Code
    // delete old keys before saving
    tmp.Printf(_T("%s/%s"), baseKey.c_str(), m_ID.c_str());
    cfg->DeleteSubPath(tmp);

    cfg->Write(tmp + _T("/name"),   m_Name);
    cfg->Write(tmp + _T("/parent"), m_ParentID, true);
  2) Change the code to save all of the config data instead of checking if it has changed
  3) Add a  cfg->Flush(); at the end of the function.

d) Remove the MirrorCurrentSettings() and the associated m_Mirror variables

What do people think about these changes? An I going on a tangent to a proper solution? Is this too much? Have I missed something?

The reason I am posting this is so that I do not go and spend a few days on this and find out that it was waste of time for some reason that someone else may know about.

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Config save bug query
« Reply #1 on: January 14, 2022, 09:17:05 am »
This could indeed be the right way to go...
I will also look into it a bit at the evening

Offline AndrewCot

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 678
Re: Config save bug query
« Reply #2 on: January 14, 2022, 10:00:26 am »
Thanks and I would love your feedback.

BTW: I did a quick and dirty hack in removed the one call to MirrorCurrentSettings() and the config file ballooned in size with allot of extra info being written to it, but nothings "bad" occurred that I seen or occurred in the 30 to 40 minutes of testing I did. The extra info looking like some macro "stuff", but I  have not looked at it and won't get to it for a day or two as I am slowly installing and setting up a new Linux Mint 20.3 guest.


Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Config save bug query
« Reply #3 on: January 18, 2022, 01:17:56 am »
Sry, i was not able to look at this, for now... I have to make a reminder for this....
I think it would be better to create a ticket...

Offline AndrewCot

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 678
Re: Config save bug query
« Reply #4 on: January 18, 2022, 06:35:03 am »
I have created ticket 1179.