[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: New changeset ffbb6f7334ccf54f09dcc9e760766d861928f13e by Victor Stinner in branch '3.6': bpo-29619: Do not use HAVE_LARGEFILE_SUPPORT for type conversions (GH-1666) (#3102) https://github.com/python/cpython/commit/ffbb6f7334ccf54f09dcc9e760766d861928f13e

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by STINNER Victor : -- pull_requests: +3141 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Xavier de Gaye : -- resolution: -> fixed status: open -> closed ___ Python tracker ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: New changeset 50e86033de85294d87b7e942701d456342abde8e by xdegaye in branch 'master': bpo-29619: Do not use HAVE_LARGEFILE_SUPPORT for type conversions (GH-1666). https://github.com/python/cpython/commit/50e86033de85294d87b7e942701d456342abde8e --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: I'm fine with always using unsigned long long. I don't think that it's even possible to measure the perf difference and if it's doable, I expect don't expect it to be significant... especially compared to the cost of the stat () syscall! Well, most computers

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: Good point. PyLong_FromLong() is faster than PyLong_FromLongLong(), IMO the fastest should be used when it is possible. PyLong_FromUnsignedLong() and PyLong_FromUnsignedLongLong() are almost identical, they implement the same algorithm. The first one uses

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Martin Panter added the comment: What is the advantage of compiling calls to both Long and LongLong converters? Isn’t it simpler to blindly call PyLong_FromUnsignedLongLong or similar? -- nosy: +martin.panter ___ Python tracker

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: PR 1666 added. With Unified Headers introduced in android-ndk-r14, the size of off_t for 32 bits Android devices is 8 instead of 4 and the cross-compilation of Python on Android does not fail in this case, see msg293956. --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Xavier de Gaye : -- pull_requests: +1761 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: Please propose the change as a PR. -- ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: > The same kind of fix could also be applied elsewhere in posixmodule.c By also adding a Py_BUILD_ASSERT() statement which is a welcome improvement on the previous code that allowed to pinpoint the problem on Android. --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: With the patch proposed by Victor in msg293873 the cross-compilation of posixmodule.c succeeds on android-21-x86 and android-21-x86_64. Following my suggestion in msg292174, I propose the following patch instead: diff --git a/Modules/posixmodule.c

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: > What is the type of st_ino? It's not off_t? The type of st_ino is unsigned long long as defined in sys/stat.h for x86 and off_t is defined in sys/types.h this way;, quoting: /* This historical accident means that we had a 32-bit off_t on 32-bit

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: Before 0f6d73343d342c106cda2219ebb8a6f0c4bd9b3c, the code used PyLong_FromLong((long)st->st_ino), so the change doesn't introduce a regression but detected a real bug, right? > On Android architecture 'x86' at api level 21: >sizeof(st->st_ino) = 8 > and

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: In configure.ac, HAVE_LARGEFILE_SUPPORT is defined if (off_t > long && longlong >= off_t) and thus, when it is not defined, it means that off_t <= long (since longlong < off_t is false) so an off_t should fit into a long. But on Android the size of off_t is

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: sizeof(unsigned long long) is 8 on Android x86 and HAVE_LARGEFILE_SUPPORT is undefined. According to msg280053 HAVE_LARGEFILE_SUPPORT is also undefined on Android x86_64 (and also on Linux x86_64). > Maybe the code should be simplified to always use unsigned

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: Would it work with unsigned long long? What is the value of HAVE_LARGEFILE_SUPPORT? Maybe the code should be simplified to always use unsigned long long. -- ___ Python tracker

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xavier de Gaye added the comment: The compilation of Modules/posixmodule.c fails now on Android architecture 'x86' at api level 21, after the above change 0f6d73343d342c106cda2219ebb8a6f0c4bd9b3c: /opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Xiang Zhang : -- resolution: -> fixed stage: -> resolved status: open -> closed versions: -Python 2.7, Python 3.5 ___ Python tracker

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: New changeset 0f6d73343d342c106cda2219ebb8a6f0c4bd9b3c by Victor Stinner in branch 'master': bpo-29619: Convert st_ino using unsigned integer (#557) https://github.com/python/cpython/commit/0f6d73343d342c106cda2219ebb8a6f0c4bd9b3c --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: New changeset 68d29809405dc766966b2b973b8597212fbc3dbd by Victor Stinner in branch '3.6': bpo-29619: Convert st_ino using unsigned integer (#557) (#584) https://github.com/python/cpython/commit/68d29809405dc766966b2b973b8597212fbc3dbd --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Serhiy Storchaka : -- pull_requests: -599 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Larry Hastings : -- pull_requests: +599 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by STINNER Victor : -- pull_requests: +481 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Mba added the comment: > I don't think that a filesystem produce inodes larger than 2^63-1 The problem is real: the large inode number was assigned to a file created by MacOS on a share exported via CIFS, and then stated by Linux using NFS export. --

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Serhiy Storchaka added the comment: I think it is worth to backport this at least to 3.6. -- nosy: +serhiy.storchaka ___ Python tracker ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

STINNER Victor added the comment: On Windows, _Py_attribute_data_to_stat() converts BY_HANDLE_FILE_INFORMATION to _Py_stat_struct, and then _pystat_fromstructstat() creates Python objects. The file index in BY_HANDLE_FILE_INFORMATION is made of two DWORD, so yes, it's unsigned. On Linux,

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by STINNER Victor : -- pull_requests: +458 ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Xiang Zhang added the comment: Any reason our _Py_stat_struct on Windows uses a signed type to represent st_ino? -- nosy: +eryksun, steve.dower ___ Python tracker

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Xiang Zhang : -- nosy: +xiang.zhang versions: -Python 3.3, Python 3.4 ___ Python tracker ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

Changes by Mba : -- components: -Library (Lib) ___ Python tracker ___ ___

[issue29619] st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat

New submission from Mba: ino_t is of type 'unsigned long' or 'unsigned long long'. Casting it to signed type means that negative values are incorrectly returned for large inodes. -- components: Library (Lib) messages: 288355 nosy: mba priority: normal severity: normal status: open