IMA at DeleteXMLNode

[auser]

Somebody have an Idea why this creates an Invalid Memory Access? PreviousXMLNode returns a value greater 0 which I would delete with DeleteXMLNode afterwards. I was using that PreviousXMLNode to avoid "deltenode(thisnode) -> nextnode(thisnode_is_deleted_already)"

Code: Select all

 
  *ChildNode = ChildXMLNode(*CurrentNode)
  While *ChildNode <> 0
    If FilterXML(*ChildNode,CurrentSublevel + 1,*grp_stack, username.s) = 1  
      *DeleteNode = *ChildNode       
      *ChildNode = NextXMLNode(*ChildNode) ;
      If *ChildNode 
        *DeleteNode = PreviousXMLNode(*ChildNode)
        If *DeleteNode
          DeleteXMLNode(*DeleteNode) ; <------------ IMA here
        EndIf
      Else 
        If *DeleteNode
          DeleteXMLNode(*DeleteNode)
          Break
        EndIf 
      EndIf 
    Else 
      *ChildNode = NextXMLNode(*ChildNode)
    EndIf 
  Wend  

[Kiffi]

@auser: can you provide a simple xml to reproduce the IMA?

Greetings ... Kiffi

[auser]

The IMA is sporadically. It works for example 100 times... then it crashes. I start the app again (with the same file) and it works again. At first I've done a deltexmlnode and then used nextxmlnode on that (already deleted) one... it even worked most time but then it crashed with IMA as well (so I thought it's maybe bad to use a node for NextXMLNode(*AlreadyDeleted) that is deleted in a previous step already). But it seems (even when it tastes somewhat wrong to me anyway) that this was not the real problem. The XML is basicaly structured in that way:

Code: Select all

<?xml version="1.0" encoding="utf-8"?>
<main>
	<maingroup name="Example" group="testuser">
		<subentry name="Example" group="testuser">
			<server name="example" group="testuser">
				<type>example</type>
			</server>
		</subentry>				
	</maingroup>
</main>

[kenmo]

I feel like we're missing some info.

Is that original code inside a recursive procedure? If so, are you using Protected variables? If not, what does FilterXML() do?

It is definitely a good idea to grab the Next node pointer before deleting the current node. I'm not sure why you have this line though, it seems unnecessary (though harmless...):

Code: Select all

*DeleteNode = PreviousXMLNode(*ChildNode)

Other than that, unless anyone else is aware of any bug with DeleteXMLNode(), I guess we would need a reproducible example of the crash...

I'm using the XML library extensively in my current project, including recursive parsing/cleanup procedures, and have not encountered any crashes. Are you using the latest PB 4.60?

[auser]

kenmo: Sorry, yes there is more (and yes it's recursive)... but since PreviousXMLNode returned non-zero I thought it's maybe enough. I don't think that "*DeleteNode = PreviousXMLNode(*ChildNode)" is useless because I check if that value is nonzero one line below. I'm using PB 4.51... I had the crash twice on linux yesterday once and today once (after changing to PreviousXMLNode) - but did not found the crash on Windows so far (anyway... I've just started testing on windows today and it even works on Linux lots of times).

Code: Select all

Procedure FilterXML(*CurrentNode,CurrentSublevel,*grp_stack, username.s)
  Protected *ChildNode
  Protected *DeleteNode
  Protected keepalive.i
  Protected retval
  Protected parse.s = ""
  Protected x.i = 0
  Protected fields.i = 0
  
  If CurrentSublevel = 0
    keepalive = 1
  EndIf 
  
  If XMLNodeType(*CurrentNode) = #PB_XML_Normal
    If ExamineXMLAttributes(*CurrentNode)
      While NextXMLAttribute(*CurrentNode)
        Select XMLAttributeName(*CurrentNode)
          Case "user", "owner", "admin"
            parse.s = XMLAttributeValue(*CurrentNode)
            fields = CountString(parse.s,",")
            If fields = 0
              fields = 1
            EndIf 
            For x = 1 To fields
              If Trim(StringField(parse.s,x,",")) = username.s
                keepalive = 1
              EndIf 
            Next 
          Case "group", "owner_grp", "admin_grp"
            parse.s = XMLAttributeValue(*CurrentNode)
            fields = CountString(parse.s,",")
            If fields = 0
              fields = 1
            EndIf 
            For x = 1 To fields
              If IsInGroup(Trim(StringField(parse.s,x,",")),*grp_stack)
                keepalive = 1
              EndIf 
            Next 
        EndSelect 
      Wend
    EndIf 
  EndIf 
  
  If keepalive = 0
    ProcedureReturn(1)
  EndIf 
  
  If CurrentSublevel = 3 ; no permission checks below that sublevel anymore
    ProcedureReturn(0)
  EndIf 
  
  *ChildNode = ChildXMLNode(*CurrentNode)
  While *ChildNode <> 0
    If FilterXML(*ChildNode,CurrentSublevel + 1,*grp_stack, username.s) = 1
      *DeleteNode = *ChildNode
      *ChildNode = NextXMLNode(*ChildNode)
      If *ChildNode
        *DeleteNode = PreviousXMLNode(*ChildNode)
        If *DeleteNode
          DeleteXMLNode(*DeleteNode)
        EndIf
      Else
        If *DeleteNode
          DeleteXMLNode(*DeleteNode)
          Break
        EndIf 
      EndIf 
    Else 
      *ChildNode = NextXMLNode(*ChildNode)
    EndIf 
  Wend  
  
EndProcedure 

[kenmo]

Oh... maybe it's a bug in the Linux version of the XML library? I've only been using it on Windows.

I haven't looked over your function yet but I will try to.

I just meant that

Code: Select all

*DeleteNode = PreviousXMLNode(*ChildNode)

was unnecessary because it immediately follows

Code: Select all

*DeleteNode = *ChildNode       
*ChildNode = NextXMLNode(*ChildNode) 

so ideally, *DeleteNode should already have the same correct pointer, right?

[auser]

so ideally, *DeleteNode should already have the same correct pointer, right?

...mhhh... - yes I think you are right, thanks for pointing me to that. Anyway I'm afraid that would not make a difference at all. But for sure I would change it (and hope )

Greetings,
auser

[auser]

I think I've found the bug and created a report (it may be very well even the case on Win64bit and seems to only occur if your XMLtree got a big returnvalue via #PB_Any in CatchXML - maybe even LoadXML): http://www.purebasic.fr/english/viewtop ... 23&t=47982

Greetings,
auser