View Issue Details

IDProjectCategoryView StatusLast Update
0026112LazarusLCLpublic2014-09-02 11:00
ReporterLuca Olivetti Assigned ToJuha Manninen  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.2 
Summary0026112: IniPropStorage wipes configuration if collection is used
DescriptionDue to a faulty implementation of IsWild used in DoEraseSection (copied verbatim from strutils), all settings in the main section will be wiped if is there a collection in the SessionProperties of the form (e.g. a TMemo):

  DoEraseSection('ajustes.Memo1_lines')

will wipe section 'ajustes' since IsWild('ajustes','ajustes.Memo1_lines.*',false) returns True.
This is a really nasty bug since it wipes the configuration file, introduced when bug 0023644 (since the bug failed to implement DoEraseSection, but at least it kept the configuration file intact albeit with useless data).
Steps To ReproduceRun the attached project, change value of the spinedit, push the button or close the project.
The value of the spinedit isn't saved in the ini file.
Additional InformationNote that this behaviour is dependent on the order of the SessionProperties (i.e., a value that comes after the last collection will be preserved, since it will be written again after wiping the main section).

Why is the IsWild needed? Wouldn't it be enough to just wipe the specified section?
And shouldn't the comparison be case insensitive?
TagsNo tags attached.
Fixed in Revisionr45077
LazTarget-
Widgetset
Attached Files

Relationships

related to 0023644 closedJuha Manninen Patches [patch] Patches for property storage classes. 
related to 0025494 resolvedMichael Van Canneyt FPC [strutils] Function isWild fail in some cases 

Activities

Luca Olivetti

2014-05-05 15:49

reporter  

published.zip (2,159 bytes)

Luca Olivetti

2014-05-05 15:56

reporter  

inipropstorage.patch (4,715 bytes)   
--- C:/Documents and Settings/luca/Configuraci´┐Żn local/Temp/inipropstorage.pas-revBASE.svn000.tmp.pas	Fri May 24 20:30:06 2013
+++ C:/laz_1_2/lcl/inipropstorage.pas	Mon May 05 15:53:22 2014
@@ -68,142 +68,10 @@
 procedure Register;
 begin
   RegisterComponents('Misc',[TIniPropStorage]);
 end;
 
-{ Should move to strutils when 1.9.6 is out. }
-
-function FindPart(const HelpWilds, InputStr: string): Integer;
-
-var
-  I, J: Integer;
-  Diff: Integer;
-
-begin
-  I := Pos('?', HelpWilds);
-  if I = 0 then begin
-    { if no '?' in HelpWilds }
-    Result := Pos(HelpWilds, InputStr);
-    Exit;
-  end;
-  { '?' in HelpWilds }
-  Diff := Length(InputStr) - Length(HelpWilds);
-  if Diff < 0 then begin
-    Result := 0;
-    Exit;
-  end;
-  { now move HelpWilds over InputStr }
-  for I := 0 to Diff do begin
-    for J := 1 to Length(HelpWilds) do begin
-      if (InputStr[I + J] = HelpWilds[J]) or
-        (HelpWilds[J] = '?') then
-      begin
-        if J = Length(HelpWilds) then begin
-          Result := I + 1;
-          Exit;
-        end;
-      end
-      else Break;
-    end;
-  end;
-  Result := 0;
-end;
-
-
-
-function IsWild(InputStr, Wilds: string; IgnoreCase: Boolean): Boolean;
-
- function SearchNext(var Wilds: string): Integer;
- { looking for next *, returns position and string until position }
- begin
-   Result := Pos('*', Wilds);
-   if Result > 0 then Wilds := Copy(Wilds, 1, Result - 1);
- end;
-
-var
-  CWild, CInputWord: Integer; { counter for positions }
-  I, LenHelpWilds: Integer;
-  MaxInputWord, MaxWilds: Integer; { Length of InputStr and Wilds }
-  HelpWilds: string;
-begin
-  if Wilds = InputStr then begin
-    Result := True;
-    Exit;
-  end;
-  repeat { delete '**', because '**' = '*' }
-    I := Pos('**', Wilds);
-    if I > 0 then
-      Wilds := Copy(Wilds, 1, I - 1) + '*' + Copy(Wilds, I + 2, MaxInt);
-  until I = 0;
-  if Wilds = '*' then begin { for fast end, if Wilds only '*' }
-    Result := True;
-    Exit;
-  end;
-  MaxInputWord := Length(InputStr);
-  MaxWilds := Length(Wilds);
-  if IgnoreCase then begin { upcase all letters }
-    InputStr := AnsiUpperCase(InputStr);
-    Wilds := AnsiUpperCase(Wilds);
-  end;
-  if (MaxWilds = 0) or (MaxInputWord = 0) then begin
-    Result := False;
-    Exit;
-  end;
-  CInputWord := 1;
-  CWild := 1;
-  Result := True;
-  repeat
-    if InputStr[CInputWord] = Wilds[CWild] then begin { equal letters }
-      { goto next letter }
-      Inc(CWild);
-      Inc(CInputWord);
-      Continue;
-    end;
-    if Wilds[CWild] = '?' then begin { equal to '?' }
-      { goto next letter }
-      Inc(CWild);
-      Inc(CInputWord);
-      Continue;
-    end;
-    if Wilds[CWild] = '*' then begin { handling of '*' }
-      HelpWilds := Copy(Wilds, CWild + 1, MaxWilds);
-      I := SearchNext(HelpWilds);
-      LenHelpWilds := Length(HelpWilds);
-      if I = 0 then begin
-        { no '*' in the rest, compare the ends }
-        if HelpWilds = '' then Exit; { '*' is the last letter }
-        { check the rest for equal Length and no '?' }
-        for I := 0 to LenHelpWilds - 1 do begin
-          if (HelpWilds[LenHelpWilds - I] <> InputStr[MaxInputWord - I]) and
-            (HelpWilds[LenHelpWilds - I]<> '?') then
-          begin
-            Result := False;
-            Exit;
-          end;
-        end;
-        Exit;
-      end;
-      { handle all to the next '*' }
-      Inc(CWild, 1 + LenHelpWilds);
-      I := FindPart(HelpWilds, Copy(InputStr, CInputWord, MaxInt));
-      if I= 0 then begin
-        Result := False;
-        Exit;
-      end;
-      CInputWord := I + LenHelpWilds;
-      Continue;
-    end;
-    Result := False;
-    Exit;
-  until (CInputWord > MaxInputWord) or (CWild > MaxWilds);
-  { no completed evaluation }
-  if CInputWord <= MaxInputWord then Result := False;
-  if (CWild <= MaxWilds) and (Wilds[MaxWilds] <> '*') then Result := False;
-end;
-
-
-
 { TCustomIniPropStorage }
 
 function TCustomIniPropStorage.IniFileClass: TIniFileClass;
 begin
   Result:=TIniFile;
@@ -275,13 +143,12 @@
 begin
   Lines := TStringList.Create;
   try
     FInifile.ReadSections(Lines);
     for I := 0 to Lines.Count - 1 do begin
-      if (Lines[I] = ARootSection) or
-        (IsWild(Lines[I], ARootSection + '.*', False) or
-        IsWild(Lines[I], ARootSection + '\*', False)) then
+      if SameText(Lines[I],ARootSection) or
+         SameText(Copy(Lines[i],1,Length(ARootSection)), ARootSection) then
         FInifile.EraseSection(Lines[I]);
     end;
   finally
     Lines.Free;
   end;
inipropstorage.patch (4,715 bytes)   

Luca Olivetti

2014-05-05 15:59

reporter   ~0074783

I uploaded a patch that doesn't use IsWild and compares the section case insensitive. If case sensitivity is necessary (which I doubt), it's just a matter of changing the SameText with a =.
And if the deletion of partial matches isn't necessary (I don't know if it is) the DoEraseSection could be reduced to

begin
 FIniFile.EraseSection(ARootSection)
end

Luca Olivetti

2014-05-05 16:10

reporter   ~0074784

In bug 0025494 there's a fix for IsWild (though for this simple case it's overkill), which is not included in fpc 2.6.4

Luca Olivetti

2014-05-05 20:37

reporter   ~0074790

The second comparison in the patch is wrong.

It should probably be

SameText(Copy(Lines[i],1,Length(ARootSection)+1), ARootSection+'.')

and another one added for '\' (but is it really needed?).

Luca Olivetti

2014-05-05 20:51

reporter   ~0074791

According to the wikipedia and http://msdn.microsoft.com/en-us/library/ms724353.aspx section and property names are not case sensitive.
The fpc implementation of TInifile has a CaseSensitive property (not initialized, hence false by default) so SameText should be the correct approach.

Luca Olivetti

2014-05-08 15:43

reporter  

inipropstorage-2.patch (4,721 bytes)   
--- C:/Documents and Settings/luca/Configuraci´┐Żn local/Temp/inipropstorage.pas-revBASE.svn001.tmp.pas	Fri May 24 20:30:06 2013
+++ C:/laz_1_2/lcl/inipropstorage.pas	Thu May 08 15:34:06 2014
@@ -68,142 +68,10 @@
 procedure Register;
 begin
   RegisterComponents('Misc',[TIniPropStorage]);
 end;
 
-{ Should move to strutils when 1.9.6 is out. }
-
-function FindPart(const HelpWilds, InputStr: string): Integer;
-
-var
-  I, J: Integer;
-  Diff: Integer;
-
-begin
-  I := Pos('?', HelpWilds);
-  if I = 0 then begin
-    { if no '?' in HelpWilds }
-    Result := Pos(HelpWilds, InputStr);
-    Exit;
-  end;
-  { '?' in HelpWilds }
-  Diff := Length(InputStr) - Length(HelpWilds);
-  if Diff < 0 then begin
-    Result := 0;
-    Exit;
-  end;
-  { now move HelpWilds over InputStr }
-  for I := 0 to Diff do begin
-    for J := 1 to Length(HelpWilds) do begin
-      if (InputStr[I + J] = HelpWilds[J]) or
-        (HelpWilds[J] = '?') then
-      begin
-        if J = Length(HelpWilds) then begin
-          Result := I + 1;
-          Exit;
-        end;
-      end
-      else Break;
-    end;
-  end;
-  Result := 0;
-end;
-
-
-
-function IsWild(InputStr, Wilds: string; IgnoreCase: Boolean): Boolean;
-
- function SearchNext(var Wilds: string): Integer;
- { looking for next *, returns position and string until position }
- begin
-   Result := Pos('*', Wilds);
-   if Result > 0 then Wilds := Copy(Wilds, 1, Result - 1);
- end;
-
-var
-  CWild, CInputWord: Integer; { counter for positions }
-  I, LenHelpWilds: Integer;
-  MaxInputWord, MaxWilds: Integer; { Length of InputStr and Wilds }
-  HelpWilds: string;
-begin
-  if Wilds = InputStr then begin
-    Result := True;
-    Exit;
-  end;
-  repeat { delete '**', because '**' = '*' }
-    I := Pos('**', Wilds);
-    if I > 0 then
-      Wilds := Copy(Wilds, 1, I - 1) + '*' + Copy(Wilds, I + 2, MaxInt);
-  until I = 0;
-  if Wilds = '*' then begin { for fast end, if Wilds only '*' }
-    Result := True;
-    Exit;
-  end;
-  MaxInputWord := Length(InputStr);
-  MaxWilds := Length(Wilds);
-  if IgnoreCase then begin { upcase all letters }
-    InputStr := AnsiUpperCase(InputStr);
-    Wilds := AnsiUpperCase(Wilds);
-  end;
-  if (MaxWilds = 0) or (MaxInputWord = 0) then begin
-    Result := False;
-    Exit;
-  end;
-  CInputWord := 1;
-  CWild := 1;
-  Result := True;
-  repeat
-    if InputStr[CInputWord] = Wilds[CWild] then begin { equal letters }
-      { goto next letter }
-      Inc(CWild);
-      Inc(CInputWord);
-      Continue;
-    end;
-    if Wilds[CWild] = '?' then begin { equal to '?' }
-      { goto next letter }
-      Inc(CWild);
-      Inc(CInputWord);
-      Continue;
-    end;
-    if Wilds[CWild] = '*' then begin { handling of '*' }
-      HelpWilds := Copy(Wilds, CWild + 1, MaxWilds);
-      I := SearchNext(HelpWilds);
-      LenHelpWilds := Length(HelpWilds);
-      if I = 0 then begin
-        { no '*' in the rest, compare the ends }
-        if HelpWilds = '' then Exit; { '*' is the last letter }
-        { check the rest for equal Length and no '?' }
-        for I := 0 to LenHelpWilds - 1 do begin
-          if (HelpWilds[LenHelpWilds - I] <> InputStr[MaxInputWord - I]) and
-            (HelpWilds[LenHelpWilds - I]<> '?') then
-          begin
-            Result := False;
-            Exit;
-          end;
-        end;
-        Exit;
-      end;
-      { handle all to the next '*' }
-      Inc(CWild, 1 + LenHelpWilds);
-      I := FindPart(HelpWilds, Copy(InputStr, CInputWord, MaxInt));
-      if I= 0 then begin
-        Result := False;
-        Exit;
-      end;
-      CInputWord := I + LenHelpWilds;
-      Continue;
-    end;
-    Result := False;
-    Exit;
-  until (CInputWord > MaxInputWord) or (CWild > MaxWilds);
-  { no completed evaluation }
-  if CInputWord <= MaxInputWord then Result := False;
-  if (CWild <= MaxWilds) and (Wilds[MaxWilds] <> '*') then Result := False;
-end;
-
-
-
 { TCustomIniPropStorage }
 
 function TCustomIniPropStorage.IniFileClass: TIniFileClass;
 begin
   Result:=TIniFile;
@@ -275,13 +143,12 @@
 begin
   Lines := TStringList.Create;
   try
     FInifile.ReadSections(Lines);
     for I := 0 to Lines.Count - 1 do begin
-      if (Lines[I] = ARootSection) or
-        (IsWild(Lines[I], ARootSection + '.*', False) or
-        IsWild(Lines[I], ARootSection + '\*', False)) then
+      if SameText(Lines[I],ARootSection) or
+         SameText(Copy(Lines[i],1,Length(ARootSection)+1), ARootSection+'.') then
         FInifile.EraseSection(Lines[I]);
     end;
   finally
     Lines.Free;
   end;
inipropstorage-2.patch (4,721 bytes)   

Luca Olivetti

2014-05-08 15:44

reporter   ~0074852

Uploaded new patch with the fix proposed in comment 74790

Juha Manninen

2014-05-18 15:22

developer   ~0075085

I applied the 2. patch. Your reasoning is solid and valid. Thanks.
Please test and close if OK.

BTW, the patch has weird paths but I could apply it using "patch -p2".

Luca Olivetti

2014-05-18 17:23

reporter   ~0075092

I consider it as already tested (since I'm using the patch myself) so I'm closing it.
Regarding the paths, probably because I tested under windows and that's what TortoiseSvn produces (extracting the previous version to a temp file in my profile).

Luca Olivetti

2014-09-02 10:01

reporter   ~0076839

I'm reopening this bug as a reminder to incorporate it to the fixes_1_2 branch (supposing that there will be a 1.2.6. release).

Juha Manninen

2014-09-02 10:40

developer   ~0076840

I added the revision r45077 into this page:
  http://wiki.freepascal.org/Lazarus_1.2_fixes_branch
It will be merged before 1.2.6 release.
BTW, you can also add revisions there yourself under the "Submitted by others" section.

Luca Olivetti

2014-09-02 11:00

reporter   ~0076843

Oh, I didn't know, I hope I'll remember for the next time.
Thank you.

Issue History

Date Modified Username Field Change
2014-05-05 15:48 Luca Olivetti New Issue
2014-05-05 15:49 Luca Olivetti File Added: published.zip
2014-05-05 15:56 Luca Olivetti File Added: inipropstorage.patch
2014-05-05 15:59 Luca Olivetti Note Added: 0074783
2014-05-05 16:10 Luca Olivetti Note Added: 0074784
2014-05-05 19:56 Juha Manninen Relationship added related to 0023644
2014-05-05 19:57 Juha Manninen Relationship added related to 0025494
2014-05-05 20:37 Luca Olivetti Note Added: 0074790
2014-05-05 20:51 Luca Olivetti Note Added: 0074791
2014-05-08 15:43 Luca Olivetti File Added: inipropstorage-2.patch
2014-05-08 15:44 Luca Olivetti Note Added: 0074852
2014-05-18 13:11 Juha Manninen Assigned To => Juha Manninen
2014-05-18 13:11 Juha Manninen Status new => assigned
2014-05-18 15:22 Juha Manninen Fixed in Revision => r45077
2014-05-18 15:22 Juha Manninen LazTarget => -
2014-05-18 15:22 Juha Manninen Note Added: 0075085
2014-05-18 15:22 Juha Manninen Status assigned => resolved
2014-05-18 15:22 Juha Manninen Resolution open => fixed
2014-05-18 17:23 Luca Olivetti Note Added: 0075092
2014-05-18 17:23 Luca Olivetti Status resolved => closed
2014-09-02 10:01 Luca Olivetti Note Added: 0076839
2014-09-02 10:01 Luca Olivetti Status closed => assigned
2014-09-02 10:01 Luca Olivetti Resolution fixed => reopened
2014-09-02 10:40 Juha Manninen Note Added: 0076840
2014-09-02 10:40 Juha Manninen Status assigned => resolved
2014-09-02 10:40 Juha Manninen Resolution reopened => fixed
2014-09-02 11:00 Luca Olivetti Note Added: 0076843
2014-09-02 11:00 Luca Olivetti Status resolved => closed