Fix #4570 Segfault on Property Getters/Setters (#8042)

* Fix #4570 Segfault on Property Getters/Setters

* Revert regression, response to review

* Remove print statement
This commit is contained in:
Brian Kelley
2024-12-09 08:34:40 -05:00
committed by GitHub
parent 090dba9cc8
commit bd85b29d43
4 changed files with 36 additions and 19 deletions

View File

@@ -42,18 +42,18 @@ void setQuery(QueryAtom *self, const QueryAtom *other) {
}
template <class T>
void AtomSetProp(const Atom *atom, const char *key, const T &val) {
void AtomSetProp(const Atom *atom, const std::string &key, const T &val) {
// std::cerr<<"asp: "<<atom<<" " << key<<" - " << val << std::endl;
atom->setProp<T>(key, val);
}
int AtomHasProp(const Atom *atom, const char *key) {
int AtomHasProp(const Atom *atom, const std::string &key) {
// std::cerr<<"ahp: "<<atom<<" " << key<< std::endl;
int res = atom->hasProp(key);
return res;
}
void AtomClearProp(const Atom *atom, const char *key) {
void AtomClearProp(const Atom *atom, const std::string &key) {
if (!atom->hasProp(key)) {
return;
}

View File

@@ -167,11 +167,11 @@ boost::python::dict GetPropsAsDict(const T &obj, bool includePrivate,
}
template <class RDOb, class T>
T GetProp(const RDOb *ob, const char *key) {
T GetProp(const RDOb *ob, const std::string &key) {
T res;
try {
if (!ob->getPropIfPresent(key, res)) {
PyErr_SetString(PyExc_KeyError, key);
PyErr_SetString(PyExc_KeyError, key.c_str());
throw python::error_already_set();
}
return res;
@@ -203,7 +203,7 @@ template <class RDOb>
python::object GetPyProp(const RDOb *obj, const std::string &key,
bool autoConvert) {
if (!autoConvert) {
return python::object(GetProp<RDOb, std::string>(obj, key.c_str()));
return python::object(GetProp<RDOb, std::string>(obj, key));
} else {
auto &data = obj->getDict().getData();
for (auto &rdvalue : data) {
@@ -283,20 +283,20 @@ python::object GetPyProp(const RDOb *obj, const std::string &key,
}
template <class RDOb>
int MolHasProp(const RDOb &mol, const char *key) {
int MolHasProp(const RDOb &mol, const std::string &key) {
int res = mol.hasProp(key);
// std::cout << "key: " << key << ": " << res << std::endl;
return res;
}
template <class RDOb, class T>
void MolSetProp(const RDOb &mol, const char *key, const T &val,
void MolSetProp(const RDOb &mol, const std::string &key, const T &val,
bool computed = false) {
mol.setProp(key, val, computed);
}
template <class RDOb>
void MolClearProp(const RDOb &mol, const char *key) {
void MolClearProp(const RDOb &mol, const std::string &key) {
if (!mol.hasProp(key)) {
return;
}

View File

@@ -99,7 +99,7 @@ ROMol *MolFromSmarts(python::object ismarts, bool mergeHs,
}
return static_cast<ROMol *>(newM);
}
ROMol *MolFromTPLFile(const char *filename, bool sanitize = true,
ROMol *MolFromTPLFile(const std::string &filename, bool sanitize = true,
bool skipFirstConf = false) {
RWMol *newM;
try {
@@ -126,7 +126,7 @@ ROMol *MolFromTPLBlock(python::object itplBlock, bool sanitize = true,
return static_cast<ROMol *>(newM);
}
ROMol *MolFromMolFileHelper(const char *molFilename, bool sanitize,
ROMol *MolFromMolFileHelper(const std::string &molFilename, bool sanitize,
bool removeHs, bool strictParsing) {
RWMol *newM = nullptr;
try {
@@ -156,7 +156,7 @@ ROMol *MolFromMolBlock(python::object imolBlock, bool sanitize, bool removeHs,
return static_cast<ROMol *>(newM);
}
ROMol *MolFromMolFile(const char *molFilename, bool sanitize, bool removeHs,
ROMol *MolFromMolFile(const std::string &molFilename, bool sanitize, bool removeHs,
bool strictParsing) {
RWMol *newM = nullptr;
try {
@@ -171,7 +171,7 @@ ROMol *MolFromMolFile(const char *molFilename, bool sanitize, bool removeHs,
return static_cast<ROMol *>(newM);
}
ROMol *MolFromMrvFile(const char *molFilename, bool sanitize, bool removeHs) {
ROMol *MolFromMrvFile(const std::string &molFilename, bool sanitize, bool removeHs) {
RWMol *newM = nullptr;
try {
newM = MrvFileToMol(molFilename, sanitize, removeHs);
@@ -226,7 +226,7 @@ ROMol *MolFromSVG(python::object imolBlock, bool sanitize, bool removeHs) {
return static_cast<ROMol *>(res);
}
ROMol *MolFromMol2File(const char *molFilename, bool sanitize = true,
ROMol *MolFromMol2File(const std::string &molFilename, bool sanitize = true,
bool removeHs = true, bool cleanupSubstructures = true) {
RWMol *newM;
try {
@@ -255,7 +255,7 @@ ROMol *MolFromMol2Block(std::string mol2Block, bool sanitize = true,
return static_cast<ROMol *>(newM);
}
ROMol *MolFromPDBFile(const char *filename, bool sanitize, bool removeHs,
ROMol *MolFromPDBFile(const std::string &filename, bool sanitize, bool removeHs,
unsigned int flavor, bool proximityBonding) {
RWMol *newM = nullptr;
try {
@@ -506,7 +506,7 @@ python::list MolToRandomSmilesHelper(const ROMol &mol, unsigned int numSmiles,
return pyres;
}
ROMol *MolFromPNGFile(const char *filename, python::object pyParams) {
ROMol *MolFromPNGFile(const std::string &filename, python::object pyParams) {
SmilesParserParams params;
if (pyParams) {
params = python::extract<SmilesParserParams>(pyParams);
@@ -604,7 +604,7 @@ python::object addMetadataToPNGStringHelper(python::dict pymetadata,
return retval;
}
python::object MolsFromPNGFile(const char *filename, const std::string &tag,
python::object MolsFromPNGFile(const std::string &filename, const std::string &tag,
python::object pyParams) {
SmilesParserParams params;
if (pyParams) {
@@ -645,7 +645,7 @@ python::tuple MolsFromPNGString(python::object png, const std::string &tag,
return python::tuple(res);
}
python::object MolsFromCDXMLFile(const char *filename, bool sanitize,
python::object MolsFromCDXMLFile(const std::string &filename, bool sanitize,
bool removeHs) {
std::vector<std::unique_ptr<RWMol>> mols;
try {

View File

@@ -8252,7 +8252,24 @@ M END
chain2mol = Chem.SplitMolByPDBChainId(mol)
self.assertEqual(len(chain2mol), 1)
self.assertIn("", chain2mol)
def testGithub4570(self):
# test sending None to get/set prop
m = Chem.Mol()
# these should not seg fault
for func in ["HasProp", "GetProp", "GetIntProp", "GetDoubleProp"]:
try:
getattr(m, func)(None)
except Exception as e:
assert "Python argument types" in str(e), f"{func}: {str(e)}"
for func, val in [("SetProp", ""), ("SetIntProp", 1), ("SetDoubleProp", 1.0)]:
try:
getattr(m, func)(None, val)
except Exception as e:
assert "Python argument types" in str(e), f"{func}: {str(e)}"
if __name__ == '__main__':
if "RDTESTCASE" in os.environ:
suite = unittest.TestSuite()