-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[BUG] Posix time_t
isn't always long
#6142
Comments
Thanks for the report. Since these are Could you clarify where exactly this hurts? Do you have a short code example that exposes this? |
@scoder Sorry I don't have a reduced example currently, but I can try producing one if desired. The failure I was seeing in Ubuntu, which now switched to 64-bit time_t on 32-bit ARM is pyfuse3. pyfuse3 seems to rely on the struct stat definition mentioned above. It reads struct stat's incorrectly unless I patch cython to use time_t as long long on armhf |
The error that you see might actually be due to code in pyfuse3, not Cython. They define access macros that explicitly return cdef extern from "macros.c" nogil:
long GET_BIRTHTIME(struct_stat* buf)
long GET_ATIME_NS(struct_stat* buf)
long GET_CTIME_NS(struct_stat* buf)
long GET_MTIME_NS(struct_stat* buf)
long GET_BIRTHTIME_NS(struct_stat* buf) https://github.com/libfuse/pyfuse3/blob/9cd9699a8c2f4a83f5228feb1211585eaf49a621/src/pyfuse3/macros.pxd#L14-L19 That said, |
Hmm. I take that back. The comment at the top of cython/Cython/Includes/posix/types.pxd Lines 1 to 11 in 16994b9
So, not sure what to do here. The declarations are actually ok and work, but the question is what users expect. If they want to be platform independent, they need to either use To me, it seems best to use |
long long is also problematic because some platforms (i386) on some distros still use 32-bit time_t but |
Anyway, could you please report the issue to pyfuse3, so that they can solve it on their side? |
So do you believe this is only related to the wrong return types of the pyfuse3 macros? Is the struct defined in Cython/Includes/posix/stat.pxd never used as a layout for accessing platform provided struct stats? My errors seems to have went away by changing the cython time_t type without touching pyfuse3 yesterday. |
So do you believe this is only related to the wrong return types of the pyfuse3 macros?
No, I only saw the wrong definitions and considered them a clear source of bugs. I did not validate that they actually have a problematic effect in the current implementation.
Is the struct defined in Cython/Includes/posix/stat.pxd never used as a layout for accessing platform provided struct stats?
Struct access is always resolved at C compile time, which sees the platform specific definition. Cython does not even need to know all fields in the struct, it's happy if you declare those that the code uses.
My errors seems to have went away by changing the cython time_t type without touching pyfuse3 yesterday.
I cannot comment on this because I do not know exactly how pyfuse3 uses the declarations.
|
Describe the bug
Cython/Includes/posix/types.pxd hard-codes time_t as long, which isn't correct on 32-bit Linux using 64-bit time_t.
This results in an incorrect struct stat layout specified by Cython/Includes/posix/stat.pxd, leading to much sadness, undefined behavior and broken programs.
The text was updated successfully, but these errors were encountered: