-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
AIX compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed #76571
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
Comments
current level: commit 4b96593 (HEAD -> master, upstream/master, origin/master, origin/HEAD) Build message: Details (note rewrite from listing for line 5514)
"./Modules/posixmodule.c", line 5514.11: 1506-131 (W) Explicit dimension specification or initializer required for an auto or static array. |
OOps - wrong error! It is the new fsid variable! michael@x071:[/data/prj/python/git/python3-3.7.0.a3]xlc_r -DNDEBUG -O -I/opt/include -O2 -qmaxmem=-1 -qarch=pwr5 -I. -I./Include -I/opt/in>
"./Modules/posixmodule.c", line 9328.64: 1506-280 (S) Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed |
FYI: from /usr/include/types.h /* typedef for the File System Identifier (fsid). This must correspond
* to the "struct fsid" structure in _ALL_SOURCE below.
*/
typedef struct fsid_t {
#ifdef __64BIT_KERNEL
unsigned long val[2];
#else /* __64BIT_KERNEL */
#ifdef _ALL_SOURCE
unsigned int val[2];
#else /* _ALL_SOURCE */
unsigned int __val[2];
#endif /* _ALL_SOURCE */
#endif /* __64BIT_KERNEL */
} fsid_t; And, currently I am building in 32-bit mode, with a 64BIT kernel (AIX 6.1) - so I expect it to be 2 unsigned_longs. However, I am not smart enough to find a solution: I tried:
PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong((unsigned long) st.f_fsid)); and PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong((unsigned long *) st.f_fsid)); Both return: "./Modules/posixmodule.c", line 9328.80: 1506-117 (S) Operand must be a scalar type. |
The PPC64 AIX 3.x Python buildbot (http://buildbot.python.org/all/#/builders/10) has been failing upon this same error for over a month. Can you please try with: PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid.val[0])); Or if statvfs.f_fsid is a pointer to fsid_t, try this instead: PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid->val[0])); |
Well, replied via email response - but it did not show up here. You suggestion worked. I'll start a PR and we can see if your suggestion also works for all the other platforms. |
Michael Felt answered by email:
Right, f_fsid has been added only few days ago by bpo-32143. |
Did some additional research. The code can work asis when _ALL_SOURCE is undefined. Or, in other words - there does not have to be a variable syntax issue or separate code for AIX. As you read this - please remember that since issue bpo-32143 was 'resolved' AIX cannot compile. describes _ALL_SOURCE as a macro for AIX3. Looking at AIX: /usr/include/sys/types.h - by undefining _ALL_SOURCE the typedefs in the header files are equivalent enough that the new code added for os.statvfs(path) is compiled by both gcc and xlc. As examples (from Centos) And from AIX:
(Note - a variation of the text above was in the pull request #4972. Per reviewer request - this is removed and a discussion is now open here.) Thank you for your comments. |
Can you please post the definition of the statvfs structure on AIX. |
struct statvfs {
ulong_t f_bsize; /* preferred file system block size */
ulong_t f_frsize; /* fundamental file system block size */
fsblkcnt_t f_blocks; /* total # of blocks of f_frsize in fs */
fsblkcnt_t f_bfree; /* total # of free blocks */
fsblkcnt_t f_bavail; /* # of blocks available to non super user */
fsfilcnt_t f_files; /* total # of file nodes (inode in JFS) */
fsfilcnt_t f_ffree; /* total # of free file nodes */
fsfilcnt_t f_favail; /* # of nodes available to non super user */
#ifdef _ALL_SOURCE
fsid_t f_fsid; /* file system id */
#else
ulong_t f_fsid; /* file system id */
#ifndef __64BIT__
ulong_t f_fstype; /* file system type */
#endif
#endif /* _ALL_SOURCE */
char f_basetype[_FSTYPSIZ]; /* Filesystem type name (eg. jfs) */
ulong_t f_flag; /* bit mask of flags */
ulong_t f_namemax; /* maximum filename length */
char f_fstr[32]; /* filesystem-specific string */
ulong_t f_filler[16];/* reserved for future use */
}; |
/* typedef for the File System Identifier (fsid). This must correspond
* to the "struct fsid" structure in _ALL_SOURCE below.
*/
typedef struct fsid_t {
#ifdef __64BIT_KERNEL
unsigned long val[2];
#else /* __64BIT_KERNEL */
#ifdef _ALL_SOURCE
unsigned int val[2];
#else /* _ALL_SOURCE */
unsigned int __val[2];
#endif /* _ALL_SOURCE */
#endif /* __64BIT_KERNEL */
} fsid_t;
#ifdef _KERNEL
typedef struct fsid32_t {
unsigned int val[2];
} fsid32_t;
#endif /* __64BIT_KERNEL */
typedef struct fsid64_t {
#if defined(_ALL_SOURCE) && (defined(__64BIT__) || defined(_LONG_LONG))
uint64_t val[2];
#else /* _ALL_SOURCE */
uint32_t __val[4];
#endif /* _ALL_SOURCE */
} fsid64_t;
/* typedef for the File System Identifier (fsid) */
struct fsid {
#ifndef __64BIT_KERNEL
unsigned int val[2];
#else
unsigned long val[2];
#endif
}; |
The _ALL_SOURCE feature test macro is set by the AC_USE_SYSTEM_EXTENSIONS macro in configure.ac. The _ALL_SOURCE feature test macro is defined here: https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.1.0/com.ibm.zos.v2r1.bpxbd00/ftms.htm |
_ALL_SOURCE is overkill. It probably is too big a club for this regression. However, the AIX header definition of fsid compatible with the current Python posixmodule.c code is bracketed by _ALL_SOURCE. AFAICT, the change to posixmodule.c made assumptions about fsid based on Linux and not required by POSIX. This didn't simply introduce a testsuite regression, but broke the build on AIX. The posixmodule.c code should be corrected or should be reverted. |
The following patch may be less invasive and more explicit: diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 38b6c80e6b..e0bb4ba869 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -9325,7 +9325,11 @@ _pystatvfs_fromstructstatvfs(struct statvfs st) {
PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
#endif
+#if defined(_AIX) && defined(_ALL_SOURCE)
+ PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid.val[0]));
+#else
PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid));
+#endif
if (PyErr_Occurred()) {
Py_DECREF(v);
return NULL; |
On 03/01/2018 14:41, David Edelsohn wrote:
Maybe. Clearly it is a big club. The documentation - if you can find any
I do not know 'why' AIX and Linux differ in the way they name things. I IMHO: 15+ years ago IBM (AIX) worked to find a method to simplify Here is where I read that _ALL_SOURCE is for AIX3. I can neither deny I considered it 'interesting' that <sys/types.h> at least talks a bit
Maybe reverting the change is better than using the "big club". But, IMHO - this is a spelling issue, going back years. But if you use a In any case the previous issue that saw adding fsid as a solution was In short - I am just a messenger - you are the experts. :)
|
Ignore my last comment - I have a headache. If I could edit/delete it I would. aka "reset 2018 - here I come!" |
On 03/01/2018 18:03, Xavier de Gaye wrote:
|
Thanks Michael for your contribution in fixing this issue. |
A compilation error is a blocking bug. It is short and short, it can be |
The AIX buildbots has been exhibiting testsuite failures, but not build (compile) failures. The buildbots do not visibly distinguish between the two cases in a strong manner. We can disable / expect failure for the few, additional testcases on AIX so that the buildbots will report "success" to make new failures more obvious. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: