search

rigglemania / pysqlcipher3

Python 3 bindings for SQLCipher
zlib License
138 stars 42 forks source link

bodge to make compile on python 3.9 #23

Closed ghost closed 3 years ago

ghost commented 3 years ago

pysqlite_row_print is not used anywhere, so this shouldn't break anything.

rigglemania commented 3 years ago

pysqlite_row_print is not used anywhere, so this shouldn't break anything.

If it's really not used anywhere, wouldn't it be better to delete the method completely. I don't really like the change as-is where the function now has unexpected behavior and potential unexpected implications.

ghost commented 3 years ago

Updated. Seems to work, although I am not sure that the trick with casting a NULL to the function pointer is truly valid. But seems to work for me.

rigglemania commented 3 years ago

You said the function wasn't being used, but based on your recent change, it clearly was. Given I'm not familiar enough to know what the implications are regarding casting to the null pointer and if that exposes any additional surface area for security vulnerabilities, I can't currently in good conscience approve/merge such a PR.

ghost commented 3 years ago

The function is not used, and using tp_print is illegal in python 3. The field that corresponds to tp_print is in python > 3.9 even replaced with some completely unrelated thing.

The exactly same structure exists in pysqlite3: https://github.com/coleifer/pysqlite3/blob/master/src/row.c (Line 237).

I replaced it with 0, instead of NULL. like they do.

rigglemania commented 3 years ago

Can you bump the library version? Otherwise, it looks good.

JayDwee commented 3 years ago

got an eta?

JayDwee commented 3 years ago

Please could this be updated on PyPi?

rigglemania commented 3 years ago

Can you let me know if the latest version works. I've been out of the Python world for a while, so had to update my local environment, and refresh myself on all the steps. I think I was able to build successfully, but don't really have a way to test this.

JayDwee commented 3 years ago

Seems to be passing all our tests still with the new version, so should be good 👍