[ wxwindows-Bugs-1685312 ] wxFileConfig::DeleteGroup not reacting as expected

[SourceForge.net]

Bugs item #1685312, was opened at 2007-03-21 16:46
Message generated for change (Settings changed) made by g00fy
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=109863&aid=1685312&group_id=9863

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Common
Group: Must fix
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: Steven Van Ingelgem (g00fy)
>Assigned to: Vadim Zeitlin (vadz)
Summary: wxFileConfig::DeleteGroup not reacting as expected

Initial Comment:
Please find the proof of concept attached hereby (against minimal sample).

Basically it is if you do a DeleteGroup on a wxFileConfig, and then add new items into the just deleted group, that you get rather weird results.

Expected results:

Step 1:
[Config]
A=1
[User]
U0=test-value
U1=test-value
U2=test-value
U3=test-value
U4=test-value

Step 2:
[Config]
A=1

Step 3:
[Config]
A=1
[User]
U0=test-value2

==> Step 3 gives:
[Config]
[User]
U0=test-value2
A=1



Greetz,
Steven

----------------------------------------------------------------------

>Comment By: Steven Van Ingelgem (g00fy)
Date: 2007-04-18 13:21

Message:
Logged In: YES 
user_id=569271
Originator: YES

I think I found the problem @ fileconf.cpp @ line +- 1730:
            for ( wxFileConfigLineList *pl = pLine->Prev();
-->               pl && pl != m_pLine && !m_pLastGroup;
                  pl = pl->Prev() )
            {
            ...
            }

In here it checks if we didn't encountered our heading (in my example, the
m_pLine was "[Config]", and this should be set as the last group.

My proposal is to change this as follows:
            for ( wxFileConfigLineList *pl = pLine->Prev();
-->                  pl && !m_pLastGroup;
                  pl = pl->Prev() )
            {
                // does this line belong to our subgroup?
                for ( size_t n = 0; n < nSubgroups; n++ )
                {
                    // do _not_ call GetGroupLine! we don't want to add it
to
                    // the local file if it's not already there
                    if ( m_aSubgroups[n]->m_pLine == pl )
                    {
                        m_pLastGroup = m_aSubgroups[n];
                        break;
                    }
                }

-->                if ( pl == m_pLine )
-->                  break;
            }


This way even if you encounter m_pLine it FIRST do the check if it's our
subgroup, and then breaks... (patch included)

Please let me know if this reflects a correct behaviour. Also I think this
fileconf needs rewriting ;-) It's too difficult to follow, but it's fast...
File Added: fileconf.cpp.patch

----------------------------------------------------------------------

Comment By: Steven Van Ingelgem (g00fy)
Date: 2007-04-18 11:57

Message:
Logged In: YES 
user_id=569271
Originator: YES

Hi,


After some more investigation, I noticed that the address
"(wxFileConfigGroup*)0x00be3c00" did NOT change. The m_pLastEntry still
pointed to the right entry.
But when it wants to "SetValue" of "U0=test-value2", it used the
wxFileConfigGroup at address 0x00be3fa8...

The differences you can check in attached screenshot.

Greetz,
Steven
File Added: screentje.png

----------------------------------------------------------------------

Comment By: Steven Van Ingelgem (g00fy)
Date: 2007-04-17 15:02

Message:
Logged In: YES 
user_id=569271
Originator: YES

Investigating more it seems it's going wrong somewhere here:

wxFileConfigLineList *wxFileConfigGroup::GetLastEntryLine()
-> There it thinks it doesn't have a LastEntryLine (which it should have,
namely A=1), so it adds the line "[User]" directly under the "[Config]"
header.

----------------------------------------------------------------------

Comment By: Steven Van Ingelgem (g00fy)
Date: 2007-04-17 14:58

Message:
Logged In: YES 
user_id=569271
Originator: YES

It seems this works perfect if the file doesn't exist first, but if you do
this on an already existing file, you get this erroneous behaviour.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=109863&aid=1685312&group_id=9863