classification
Title: st_ino (unsigned long long) is casted to long long in posixmodule.c:_pystat_fromstructstat
Type: behavior Stage: resolved
Components: Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, haypo, mba, serhiy.storchaka, steve.dower, xdegaye, xiang.zhang
Priority: normal Keywords:

Created on 2017-02-22 12:24 by mba, last changed 2017-04-23 17:18 by xdegaye. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 557 merged haypo, 2017-03-08 08:04
PR 584 merged haypo, 2017-03-09 16:41
Messages (10)
msg288355 - (view) Author: Mba (mba) Date: 2017-02-22 12:24
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.
msg289213 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-03-08 07:36
Any reason our _Py_stat_struct on Windows uses a signed type to represent st_ino?
msg289216 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-03-08 08:07
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, stat.st_ino type is ino_t which is unsigned too.

So I created a pull request to fix the bug, even if I don't think that a filesystem produce inodes larger than 2^63-1. Not sure if it's worth it to backport the fix to Python 2.7, 3.5 and 3.6?
msg289221 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-08 09:24
I think it is worth to backport this at least to 3.6.
msg289288 - (view) Author: Mba (mba) Date: 2017-03-09 12:52
> 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.
msg290247 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-03-24 22:37
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
msg290250 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-03-24 22:38
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
msg292141 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-04-22 22:26
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 --sysroot=/opt/android-ndk/platforms/android-21/arch-x86 -target i686-none-linux-androideabi -gcc-toolchain /opt/android-ndk/toolchains/x86-4.9/prebuilt/linux-x86_64 -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wno-unused-value -Wno-empty-body -Qunused-arguments -Wno-parentheses-equality   -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration  -IObjects -IInclude -IPython -I. -I./Include -I/opt/android-ndk/platforms/android-21/arch-x86/usr/include   -DPy_BUILD_CORE  -c ./Modules/posixmodule.c -o Modules/posixmodule.o
./Modules/posixmodule.c:1935:5: error: array size is negative
    Py_BUILD_ASSERT(sizeof(unsigned long) >= sizeof(st->st_ino));
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./Include/pymacro.h:43:15: note: expanded from macro 'Py_BUILD_ASSERT'
        (void)Py_BUILD_ASSERT_EXPR(cond);   \
              ^~~~~~~~~~~~~~~~~~~~~~~~~~
./Include/pymacro.h:40:19: note: expanded from macro 'Py_BUILD_ASSERT_EXPR'
    (sizeof(char [1 - 2*!(cond)]) - 1)
                  ^~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:1720: Modules/posixmodule.o] Error 1

The following code:
  #include <stdio.h>
  #include <sys/stat.h>

  int main()
  {
      struct stat *st;

      printf("sizeof(unsigned long): %d - sizeof(st->st_ino): %d\n",
             sizeof(unsigned long), sizeof(st->st_ino));
      return 0;
  }
prints the following result on this same system:
  sizeof(unsigned long): 4 - sizeof(st->st_ino): 8
msg292142 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-04-22 22:48
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.
msg292174 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-04-23 17:18
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 long long.
Yes. If I understand correcty, one could only use 'unsigned long long' to assert at build time and use the smallest of the two unsigned long that fits with the size of st->st_ino at runtime.
History
Date User Action Args
2017-04-23 17:18:28xdegayesetmessages: + msg292174
2017-04-22 22:48:43hayposetmessages: + msg292142
2017-04-22 22:26:39xdegayesetnosy: + xdegaye
messages: + msg292141
2017-03-24 22:58:04xiang.zhangsetstatus: open -> closed
stage: resolved
resolution: fixed
versions: - Python 2.7, Python 3.5
2017-03-24 22:38:32hayposetmessages: + msg290250
2017-03-24 22:37:58hayposetmessages: + msg290247
2017-03-18 06:32:45serhiy.storchakasetpull_requests: - pull_request599
2017-03-17 21:00:33larrysetpull_requests: + pull_request599
2017-03-09 16:41:36hayposetpull_requests: + pull_request481
2017-03-09 12:52:08mbasetmessages: + msg289288
2017-03-08 09:24:49serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg289221
2017-03-08 08:08:00hayposetnosy: + haypo
messages: + msg289216
2017-03-08 08:04:54hayposetpull_requests: + pull_request458
2017-03-08 07:36:06xiang.zhangsetnosy: + eryksun, steve.dower
messages: + msg289213
2017-02-22 12:55:43xiang.zhangsetnosy: + xiang.zhang

versions: - Python 3.3, Python 3.4
2017-02-22 12:39:48mbasetcomponents: - Library (Lib)
2017-02-22 12:24:02mbacreate