fix #39 #42
fix #39 #42
Conversation
| Uint* = int32 | ||
| Ulong* = uint32 | ||
| Ulongf* = uint32 | ||
| Uint* {.importc: "uInt", header: "<zlib.h>".} = cuint |
There was a problem hiding this comment.
Importing a type alias should not be used, it's a mess in Nim's C codegen already.
There was a problem hiding this comment.
But I don't see an alternative to it. The type is different on different platforms. It is not just that different platforms are different. Different versions changed the type behind the type alias as well. What I don't like about it is, sizeof(Uint) will be the same as cuint, so using Uint can't be seen as safe. In combination of Uint being exported, I see this as a problem.
There was a problem hiding this comment.
It's not my fault you can only see this one particular way to fix the bug.
There was a problem hiding this comment.
If you have a different solution that doesn't break compatibility with existing versions of zlib, go ahead and implement the fix. Then when you realized that the import of the type alias is actually the correct way to handle this, you can still come back merge this and clean up the parts in the C codegen that you don't like.
If you however don't plan to fix bugs in zlib anymore and you don't want to be bothered with other peoples pull request because you are too busy implementing new feauturs in Nim, then you can still archive this repository to indicate that it isn't maintained anymore.
There was a problem hiding this comment.
Well I can also simply close this PR as it's untested on Windows, ignores the fact that the existing wrapper doesn't depend on a header file, etc etc...
not tested on windows