ball icon indicating copy to clipboard operation
ball copied to clipboard

Infinite loop in SDGenerator

Open dstoeckel opened this issue 10 years ago • 2 comments

When trying to reproduce bug #536 I used BALLViews "Build from SMILES" functionality to generate the suggested example molecule:

COc1=c2c(=cc=c1)OCCCCCCCCOc1=c(c3=c4c(=c2c2=cc=cc=c32)c=cc=c4)c(=cc=c1)O

However, processing of this SMILE never terminates. Processing seems to hang in BALL::RingAnalyser::sequenceRing:

while(!ring_atoms.empty())
{
    for (std::list<Atom*>::iterator ring_it  = ring_atoms.begin();
                                    ring_it != ring_atoms.end(); ++ring_it)
    {
        if (last_atom->isBoundTo(**ring_it))
        {
            ring[++last_index] = *ring_it;
            last_atom = *ring_it;
            ring_atoms.erase(ring_it);
            break;
        }
    }
}

Termination if no ring atom could be erased:

while(!ring_atoms.empty())
{
    bool found = false;
    for (std::list<Atom*>::iterator ring_it  = ring_atoms.begin();
                                    ring_it != ring_atoms.end(); ++ring_it)
    {
        if (last_atom->isBoundTo(**ring_it))
        {
            ring[++last_index] = *ring_it;
            last_atom = *ring_it;
            ring_atoms.erase(ring_it);
            found = true;
            break;
        }
    }
    if(!found) break;
}

lead to a subsequent segmentation fault in BALL::RingAnalyser::peelNextRing_.

Edit: fix bogus termination condition

dstoeckel avatar Mar 20 '15 12:03 dstoeckel

...oh sorry: I never use the BALL SMILES parser, I would have recomended to use open babel or Marvin from ChemAxon to generate a SDFile from that SMILES string. It seems our BALL::SMILES parser has a multitude of problems. I forgot to create an issue for that.

pbrach avatar Mar 20 '15 14:03 pbrach

No need to be sorry, infinite loops simply should not happen ;-) However the problem does not seem to be in the SMILES parser, although it certainly has its flaws.

dstoeckel avatar Mar 20 '15 15:03 dstoeckel