View Issue Details

IDProjectCategoryView StatusLast Update
0034854FPCFCLpublic2019-02-18 11:43
ReporterOleg LubashinAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformi386OSWindows 32OS VersionWindows 7
Product VersionProduct Build57972 
Target Version3.2.0Fixed in Version3.3.1 
Summary0034854: TXMLConfig.Filename property is not initialized
DescriptionTXMLConfig.Filename property is not initialized with the value set in Object Inspector, when used as a component of a form.
Here is some more analysis:
http://forum.lazarus.freepascal.org/index.php/topic,43870.0.html
Steps To ReproduceAttached is project Splitter.
XMLConfig1.Filename property is set to 'config.xml'.
When compiled and run, reports SIGSEGV error.
If XMLConfig1.Filename:='config.xml' is assigned during runtime (uncomment lines 40 and 72), then there is no error reported.
TagsNo tags attached.
Fixed in Revision41332
FPCOldBugId
FPCTarget
Attached Files
  • splitter.7z (61,083 bytes)
  • xmlconf.diff (1,231 bytes)
    Index: packages/fcl-xml/src/xmlconf.pp
    ===================================================================
    --- packages/fcl-xml/src/xmlconf.pp	(revision 41290)
    +++ packages/fcl-xml/src/xmlconf.pp	(working copy)
    @@ -166,6 +166,7 @@
     begin
       F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);
       try
    +    FFileName := '';
         ReadXMLFile(Doc, AFilename);
         FFileName:=AFileName;
       finally
    @@ -398,11 +399,14 @@
     begin
       if (not ForceReload) and (FFilename = AFilename) then
         exit;
    -    
    +
       Flush;
       FreeAndNil(Doc);
       if csLoading in ComponentState then
    +  begin
    +    FFilename := AFilename;
         exit;
    +  end;
       if FileExists(AFilename) and not FStartEmpty then
         LoadFromFile(AFilename)
       else if not Assigned(Doc) then
    @@ -425,6 +429,8 @@
       if AValue <> FRootName then
       begin
         FRootName := AValue;
    +    if not (ComponentState * [csLoading,csDesigning] = []) then
    +      Exit;
         Root := Doc.DocumentElement;
         Cfg := Doc.CreateElement(AValue);
         while Assigned(Root.FirstChild) do
    @@ -475,7 +481,7 @@
     begin
       for I := Length(FPathStack)-1 downto 0 do
         FPathStack[I] := '';
    -  FElement := nil;    
    +  FElement := nil;
       FPathDirty := False;
       FPathCount := 0;
     end;
    
    xmlconf.diff (1,231 bytes)

Activities

Oleg Lubashin

2019-01-12 09:04

reporter  

splitter.7z (61,083 bytes)

jamie philbrook

2019-01-12 19:17

reporter   ~0113359

In the "XMLConfig" file at around 297 or so..
you have this
procedure TXMLConfig.DoSetFilename(const AFilename: String; ForceReload: Boolean);
begin
  if (not ForceReload) and (FFilename = AFilename) then
    exit;
    
  Flush; // should this become after the Loading State?
  FreeAndNil(Doc); //Same?
  if csLoading in ComponentState then //This code should be setting the string.
   +Begin //Added
   + FfileName := AfileName; //Added
    exit;
   +End; //Added
  if FileExists(AFilename) and not FStartEmpty then
    LoadFromFile(AFilename)
  else if not Assigned(Doc) then
    begin
    FFileName:=AFileName;
    Doc := TXMLDocument.Create;
    Doc.AppendChild(Doc.CreateElement(FRootName))
    end;
end;

procedure TXMLConfig.SetFilename(const AFilename: String);
begin
  DoSetFilename(AFilename, False);
end;

Hope that helps.

Juha Manninen

2019-01-12 21:12

reporter   ~0113362

I think the FFileName should be set right after checking if (FFilename = AFilename). Like:

  if (not ForceReload) and (FFilename = AFilename) then
    exit;
 +FFileName:=AFileName;

Anyway the code is in FPC's libs. This issue must be moved to FPC project.
You could also create a proper patch.

jamie philbrook

2019-01-12 23:48

reporter   ~0113368

Last edited: 2019-01-13 04:12

View 2 revisions

True about placement of correction and also note that there is a unneeded
check being done

Else if Not Assigned(Doc) then....

As you can see the DOC has already been Cleared to a Nil...
and with your suggestion setting the file name earlier leaves only
The last two lines which is a repeat of what happens in the Constructor.

 Looks like the complete method should be overhauled.

Update:
  I think there is somethin else wrong. If you change the Rootname in the OI
it randomly raises a Access Violation in the IDE.
 Looking at the code it appears there are no checks being done for testing in
in design mode in the SetRootName, at least I don't see any..

 I really don't think this component was designed for OI friendy..

Bart Broersma

2019-02-13 13:32

reporter   ~0114084

Setting Filename is supposed to open the given file and only after that succeeds it will update FFilename unless LoadFromFile succeeds.
LoadFromFile should maybe set FFilename to '' if it fails?

Bart Broersma

2019-02-13 14:02

reporter  

xmlconf.diff (1,231 bytes)
Index: packages/fcl-xml/src/xmlconf.pp
===================================================================
--- packages/fcl-xml/src/xmlconf.pp	(revision 41290)
+++ packages/fcl-xml/src/xmlconf.pp	(working copy)
@@ -166,6 +166,7 @@
 begin
   F:=TFileStream.Create(AFileName,fmOpenread or fmShareDenyWrite);
   try
+    FFileName := '';
     ReadXMLFile(Doc, AFilename);
     FFileName:=AFileName;
   finally
@@ -398,11 +399,14 @@
 begin
   if (not ForceReload) and (FFilename = AFilename) then
     exit;
-    
+
   Flush;
   FreeAndNil(Doc);
   if csLoading in ComponentState then
+  begin
+    FFilename := AFilename;
     exit;
+  end;
   if FileExists(AFilename) and not FStartEmpty then
     LoadFromFile(AFilename)
   else if not Assigned(Doc) then
@@ -425,6 +429,8 @@
   if AValue <> FRootName then
   begin
     FRootName := AValue;
+    if not (ComponentState * [csLoading,csDesigning] = []) then
+      Exit;
     Root := Doc.DocumentElement;
     Cfg := Doc.CreateElement(AValue);
     while Assigned(Root.FirstChild) do
@@ -475,7 +481,7 @@
 begin
   for I := Length(FPathStack)-1 downto 0 do
     FPathStack[I] := '';
-  FElement := nil;    
+  FElement := nil;
   FPathDirty := False;
   FPathCount := 0;
 end;
xmlconf.diff (1,231 bytes)

Bart Broersma

2019-02-13 14:02

reporter   ~0114087

Possible patch attached.

Michael Van Canneyt

2019-02-16 09:51

administrator   ~0114171

Fixed using the patch of Bart Broersma, many thanks !

Issue History

Date Modified Username Field Change
2019-01-12 09:04 Oleg Lubashin New Issue
2019-01-12 09:04 Oleg Lubashin File Added: splitter.7z
2019-01-12 19:17 jamie philbrook Note Added: 0113359
2019-01-12 21:12 Juha Manninen Note Added: 0113362
2019-01-12 21:13 Juha Manninen Project Lazarus => FPC
2019-01-12 23:48 jamie philbrook Note Added: 0113368
2019-01-13 04:12 jamie philbrook Note Edited: 0113368 View Revisions
2019-02-13 13:32 Bart Broersma Note Added: 0114084
2019-02-13 14:02 Bart Broersma File Added: xmlconf.diff
2019-02-13 14:02 Bart Broersma Note Added: 0114087
2019-02-15 13:53 Michael Van Canneyt Assigned To => Michael Van Canneyt
2019-02-15 13:53 Michael Van Canneyt Status new => assigned
2019-02-16 09:51 Michael Van Canneyt Fixed in Revision => 41332
2019-02-16 09:51 Michael Van Canneyt Note Added: 0114171
2019-02-16 09:51 Michael Van Canneyt Status assigned => resolved
2019-02-16 09:51 Michael Van Canneyt Fixed in Version => 3.3.1
2019-02-16 09:51 Michael Van Canneyt Resolution open => fixed
2019-02-16 09:51 Michael Van Canneyt Target Version => 3.2.0