Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.7 Support #8

Open
sscherfke opened this issue Jul 13, 2018 · 4 comments
Open

Python 3.7 Support #8

sscherfke opened this issue Jul 13, 2018 · 4 comments

Comments

@sscherfke
Copy link

The latest PR #7 compiles with Python 3.7 but when I import base92 I get a segfault.

I have not yet digged deeper into yet.

@sscherfke
Copy link
Author

sscherfke commented Jul 18, 2018

>>> import faulthandler
>>> faulthandler.enable()
>>> import base92
Fatal Python error: Segmentation fault

Current thread 0x00007fc11e5f6740 (most recent call first):
  File "<frozen importlib._bootstrap>", line 219 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 1043 in create_module
  File "<frozen importlib._bootstrap>", line 583 in module_from_spec
  File "<frozen importlib._bootstrap>", line 670 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 967 in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 983 in _find_and_load
  File "/opt/emsconda/conda-bld/base92_1531909595061/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeold_placehold_pla
cehold_placehold_placehold_/lib/python3.7/site-packages/base92/cbase92.py", line 38 in <module>
  File "<frozen importlib._bootstrap>", line 219 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 728 in exec_module
  File "<frozen importlib._bootstrap>", line 677 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 967 in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 983 in _find_and_load
  File "<frozen importlib._bootstrap>", line 219 in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1035 in _handle_fromlist
  File "/opt/emsconda/conda-bld/base92_1531909595061/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/lib/python3.7/site-packages/base92/__init__.py", line 17 in <module>
  File "<frozen importlib._bootstrap>", line 219 in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 728 in exec_module
  File "<frozen importlib._bootstrap>", line 677 in _load_unlocked
  File "<frozen importlib._bootstrap>", line 967 in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 983 in _find_and_load
  File "<stdin>", line 1 in <module>
Segmentation fault (core dumped)

@sscherfke
Copy link
Author

If I compile the c-lib with gcc -c -Wall -Werror -fpic -shared -o $PREFIX/lib/libbase92.so -Ic/src c/src/base92.c I get some more errors:

c/src/base92.c: In function ‘base92encode’:
c/src/base92.c:84:17: error: pointer targets in return differ in signedness [-Werror=pointer-sign]
                 return "~";
                 ^
c/src/base92.c: In function ‘base92decode’:
c/src/base92.c:162:9: error: pointer targets in passing argument 1 of ‘strlen’ differ in signedness [-Werror=pointer-s
ign]
         size = strlen(str);
         ^
In file included from c/src/base92.h:12:0,
                 from c/src/base92.c:7:
/usr/include/string.h:395:15: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
 extern size_t strlen (const char *__s)
               ^
c/src/base92.c:164:9: error: pointer targets in passing argument 1 of ‘strcmp’ differ in signedness [-Werror=pointer-s
ign]
         if (strcmp(str, "~") == 0 || size == 0) {
         ^
In file included from c/src/base92.h:12:0,
                 from c/src/base92.c:7:
/usr/include/string.h:140:12: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
 extern int strcmp (const char *__s1, const char *__s2)
            ^
cc1: all warnings being treated as errors

Maybe this helps.

@sscherfke
Copy link
Author

The following patch solves the problem:

diff --git a/c/src/base92.c b/c/src/base92.c
index e0208b0..d5c50f7 100644
--- a/c/src/base92.c
+++ b/c/src/base92.c
@@ -79,9 +79,9 @@ unsigned char* base92encode(unsigned char* str, int len) {
         int tmp;
         unsigned char c;
         unsigned char *res;
-
+
         if (len == 0) {
-                return "~";
+                return (unsigned char*)"~";
         }
         // precalculate how much space we need to malloc
         size = (len * 8) % 13;
@@ -159,9 +159,9 @@ unsigned char* base92decode(unsigned char* str, int* len) {
         unsigned char* res;
         unsigned long workspace;
         unsigned short wssize;
-        size = strlen(str);
+        size = strlen((char*)str);
         // handle small cases first
-        if (strcmp(str, "~") == 0 || size == 0) {
+        if (strcmp((char*)str, "~") == 0 || size == 0) {
                 res = (unsigned char*)malloc(sizeof(char) * 1);
                 res[0] = 0;
                 return res;
diff --git a/python/base92/base92_extension.c b/python/base92/base92_extension.c
index 57907cf..a9687bc 100644
--- a/python/base92/base92_extension.c
+++ b/python/base92/base92_extension.c
@@ -289,6 +289,7 @@ static PyMethodDef base92_methods[] =
 {
     {"encode", (PyCFunction)PyBase92_encode, METH_VARARGS, encode__doc__},
     {"decode", (PyCFunction)PyBase92_decode, METH_VARARGS, decode__doc__},
+    {NULL, NULL, 0, NULL}        /* Sentinel */
 };


@@ -315,17 +316,11 @@ static struct PyModuleDef base92_def = {
     "base92_extension",
     NULL,
     -1,
-    base92_methods,
-    NULL,
-    NULL,
-    NULL,
-    NULL
+    base92_methods
 };

 PyMODINIT_FUNC
 PyInit_base92_extension(void) {
-    PyObject *m = PyModule_Create(&base92_def);
-
-    return m;
+    return PyModule_Create(&base92_def);
 }
 #endif

@backbord
Copy link

Hi @sscherfke, looks good. If you'll open a PR for my fork, I'll gladly merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants