Hi Jan, Thanks for catching the problem. Your proposed solution looks reasonable to me.
As an aside: I believe that assigning an RWMol to another like you did in your first attempt should work. The fact that it doesn't is a separate bug that I will go ahead and fix. -greg On Thu, Nov 15, 2018 at 9:03 PM Jan Holst Jensen <[email protected]> wrote: > I seem to have found out. The input CROMol should not be casted to an > RWMol pointer, instead it should be casted to an ROMol pointer. > > Then I can do this: > > extern "C" char *MolGetSVG(CROMol i, unsigned int w, unsigned int h, > const char *legend, const char *params) { > // SVG routines need an RWMol since they change the > // molecule as they prepare it for drawing. We don't > // want a plain SQL function (mol_to_svg) to have > // unexpected side effects, so take a copy and render > // (and change) that. > * const ROMol *input_mol = (ROMol *)i;* > RWMol input_copy = *input_mol; > RWMol *im = &input_copy; > > ... > > and that does not crash the database and my InChI keys are no longer > changing (rarely, but happens), depending on whether I call mol_to_svg() or > not. > > I will run with this for a while. If it is stable I will make a pull > request. > > Cheers > -- Jan > > On 2018-11-15 18:33, Jan Holst Jensen wrote: > > Hi, > > I have an issue with the cartridge - mol_to_svg() changes the input > molecule so I want to make MolGetSVG() to work on a copy of the input > molecule instead. > > In Code/PgSQL/rdkit/adapter.cpp: > > extern "C" char *MolGetSVG(CROMol i, unsigned int w, unsigned int h, > const char *legend, const char *params) { > // This is the original code that makes a read-write > // "im" that is passed to the drawing routines where > // it gets changed. > RWMol *im = (RWMol *)i; > > // SVG routines need an RWMol since they change the > // molecule as they prepare it for drawing. We don't > // want a plain SQL function (mol_to_svg) to have > // unexpected side effects, so take a copy and render > // (and change) that instead. > RWMol *input_mol = (RWMol *)i; > RWMol input_copy = *input_mol; > RWMol *im = &input_copy; > > ... > > My construction of the "input_copy" object is obviously wrong, 'cause the > database crashes. What is the correct way of copying an RWMol object ? > > Cheers > -- Jan > > > _______________________________________________ > Rdkit-discuss mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/rdkit-discuss >
_______________________________________________ Rdkit-discuss mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/rdkit-discuss
