classification
Title: mknod devices can be >32 bits
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: jcea Nosy List: jcea, python-dev, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2014-12-21 22:36 by jcea, last changed 2015-01-18 09:47 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
posix_dev_t_converter-3.5.patch serhiy.storchaka, 2014-12-22 16:32 review
posix_dev_t_converter-3.5_2.patch serhiy.storchaka, 2014-12-22 21:22 review
posix_dev_t_converter-2.7.patch serhiy.storchaka, 2014-12-23 07:55 review
posix_dev_t_converter-3.4.patch serhiy.storchaka, 2014-12-23 07:55 review
Messages (8)
msg233004 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2014-12-21 22:36
Dan MacDonald told me that "os.mknod()" should accept devices >32 bits.

I wrote this code in linux 64 bits:

"""
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>

int main(void) {
    printf("%d", sizeof(dev_t));
    return 0;
}
"""

The result of running this is "8". We need a long long.

I did this change in Python 2.7 branch:

"""
diff -r f00412d32b41 Modules/posixmodule.c
--- a/Modules/posixmodule.c	Sat Dec 20 13:41:14 2014 -0600
+++ b/Modules/posixmodule.c	Sun Dec 21 23:30:00 2014 +0100
@@ -7009,9 +7009,9 @@
 {
     char *filename;
     int mode = 0600;
-    int device = 0;
+    long long device = 0;
     int res;
-    if (!PyArg_ParseTuple(args, "s|ii:mknod", &filename, &mode, &device))
+    if (!PyArg_ParseTuple(args, "s|iL:mknod", &filename, &mode, &device))
         return NULL;
     Py_BEGIN_ALLOW_THREADS
     res = mknod(filename, mode, device);
"""

Looks like this patch is trivial. Please, comment.
msg233015 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-22 09:20
This is not so easy as look at first glance. PY_LONG_LONG should be used instead of long long. And as far as this type is optional, its use should be conditional if HAVE_LONG_LONG is defined.

May be needed complex converter like _Py_Uid_Converter because dev_t on Linux is 64-bit unsigned int and can not fit in the range of 64-bit signed int.

The code for 3.x should be even more complex due to using Argument Clinic.
msg233019 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2014-12-22 13:08
Serhiy, could you consider to create a patch for 2.7 and 3.4?. I am not familiar with the details of Argument Clinic.
msg233022 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-22 16:32
Here is a patch against 3.5. Unsigned bitwise int converter is used for dev_t because dev_t can be unsigned (and it is on Linux). This is not ideal solution, there is no overflow check, but at least it should work for correct code.
msg233026 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-22 21:22
As pointed by Antoine there are other affected functions. Updated patch fixes converting of arguments or results in makedev(), major() and minor().
msg233037 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-23 07:55
And here are patches for 2.7 and 3.4 (unfortunately none of patches is applied 
clear to other version).
msg234229 - (view) Author: Roundup Robot (python-dev) Date: 2015-01-18 09:18
New changeset 7ee09e2fec13 by Serhiy Storchaka in branch '2.7':
Issue #23098: 64-bit dev_t is now supported in the os module.
https://hg.python.org/cpython/rev/7ee09e2fec13

New changeset 18703ffea2b3 by Serhiy Storchaka in branch '3.4':
Issue #23098: 64-bit dev_t is now supported in the os module.
https://hg.python.org/cpython/rev/18703ffea2b3

New changeset fe0fddd6fd21 by Serhiy Storchaka in branch 'default':
Issue #23098: 64-bit dev_t is now supported in the os module.
https://hg.python.org/cpython/rev/fe0fddd6fd21
msg234232 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-18 09:47
Committed to 2.7 with small change: stat() and makedev() should return int instead of long if possible.
History
Date User Action Args
2015-01-18 09:47:58serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg234232

stage: patch review -> resolved
2015-01-18 09:18:51python-devsetnosy: + python-dev
messages: + msg234229
2014-12-23 07:55:50serhiy.storchakasetfiles: + posix_dev_t_converter-2.7.patch, posix_dev_t_converter-3.4.patch

messages: + msg233037
2014-12-22 21:22:06serhiy.storchakasetfiles: + posix_dev_t_converter-3.5_2.patch

messages: + msg233026
2014-12-22 16:32:47serhiy.storchakasetfiles: + posix_dev_t_converter-3.5.patch
keywords: + patch
messages: + msg233022
2014-12-22 13:08:18jceasetmessages: + msg233019
2014-12-22 09:20:23serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg233015

components: + Extension Modules
type: behavior
2014-12-21 22:36:24jceacreate