Created on 2011-01-26 10:57 by pitrou, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Solaris has an additional kind of special files named doors. The standard headers define the following macro: #define S_ISDOOR(mode) (((mode)&0xF000) == 0xd000) Perhaps it would be nice to include the equivalent in the stat module. (although funnily even the "stat" command doesn't recognize them and displayed "weird file" instead: $ stat /var/run/syslog_door File: `/var/run/syslog_door' Size: 0 Blocks: 0 IO Block: 0 weird file Device: 8bc0000h/146538496d Inode: 44 Links: 1 Access: (0644/Drw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) Access: 2011-01-26 11:52:34.332492612 +0100 Modify: 2011-01-26 11:52:34.332492612 +0100 Change: 2011-01-26 11:52:34.332499428 +0100 )
Thanks for the feature request. It seems trivial to implement, but not trivial to test :). I assign this to myself. I will work on it after Mercurial migration is finished. Getting impatient :).
> Thanks for the feature request. It seems trivial to implement, but not trivial to test :). You can test against an existing file, such as "/var/run/syslog_door" (given its name, it is unlikely to be anything other than a door)
I do know, but when you are working inside a zone, I am not sure you can count of that file being always present. Syslog even could be disabled or not available inside a zone. I am thinking about how to manage OS's with no support for doors. Instead of conditionally compile the new STAT, I would rather have the function always available, but returning FALSE when the OS doesn't support doors. But what happen if you mount a Solaris filesystem in a nonsolaris machine?. Let say, a Solaris ZFS filesystem with doors, under linux/*bsd + ZFS?. So maybe would be better to not compile that function in those OS's, instead of "lying" in corner cases. Not decided yet. I accept ideas.
> I do know, but when you are working inside a zone, I am not sure you > can count of that file being always present. Syslog even could be > disabled or not available inside a zone. Then you can just skip the test. > I am thinking about how to manage OS's with no support for doors. > Instead of conditionally compile the new STAT, Lib/stat.py is a pure Python module. There is no compilation to do. You just have to add the pure Python version of the C macro. For the record, 0xd000 is (stat.S_IFSOCK + stat.S_IFIFO). > I would rather have the function always available, but returning FALSE > when the OS doesn't support doors. Right. > But what happen if you mount a Solaris filesystem in a nonsolaris > machine?. Let say, a Solaris ZFS filesystem with doors, under > linux/*bsd + ZFS?. I guess S_ISDOOR would return True.
Given the behaviour of Solaris DOORS, the flags selected are very cleverly chosen. Sensible engineering :). But now I am wondering... Which organization defines flags like S_IFSOCK or S_IFIFO?. Posix members?. I am worried about flag collision between OSs. For instance, if DOORS were a real flag defined in Solaris, how other OSs would know to not reuse that flag for some other purpose? :). Your comment is very valuable and solve my doubts about it. Implementation is truly trivial and could be supported everywhere.
> But now I am wondering... Which organization defines flags like > S_IFSOCK or S_IFIFO?. Posix members?. I am worried about flag > collision between OSs. They are defined unconditionally in Lib/stat.py. > For instance, if DOORS were a real flag defined in Solaris, how other > OSs would know to not reuse that flag for some other purpose? :). Which flag? S_ISDOOR uses an (unlikely) combination of existing flags.
Antoine, I am not talking about python, I am talking about the UNIX standarization process. In particular, how a new flag (for filesystems) in an OS can be "skipped" and not being reused for some other purpuse in other OS. Off topic for python, but curious to know anyway :).
Jesus, yes, it's totally possible that POSIX might define: (stat.S_IFSOCK + stat.S_IFIFO) to mean something other than S_IDOOR. However, the POSIX committee is always careful to respect existing usage. It's vanishingly unlikely that any attempt to do this would actually happen, since it would be obviously unimplementable on the most popular UNIX (namely Solaris).
I am looking at Linux :-). Anyway the feedback has been very useful. I will implement this as soon as mercurial switch is done.
> I am thinking about how to manage OS's with no support for doors. > Instead of conditionally compile the new STAT, I would rather have > the function always available, but returning FALSE when the OS > doesn't support doors. But what happen if you mount a Solaris > filesystem in a nonsolaris machine?. Let say, a Solaris ZFS > filesystem with doors, under linux/*bsd + ZFS?. > > So maybe would be better to not compile that function in those OS's, > instead of "lying" in corner cases. I'd rather see this exposed from the posix module than the stat module (and see the stat module deprecated in the long term). As much other stuff, it only be defined if the system actually has the functionality. sys.path.isdoor could be defined on top of it, returning False on systems that don't have doors in the first place. For testing, if S_ISDOR is available but /var/run/name_service_door is not, the test should get skipped.
Am 26.01.2011 14:14, schrieb Jesús Cea Avión: > > Jesús Cea Avión <jcea@jcea.es> added the comment: > > Antoine, I am not talking about python, I am talking about the UNIX > standarization process. In particular, how a new flag (for > filesystems) in an OS can be "skipped" and not being reused for some > other purpuse in other OS. There is no standards body defining the S_IFMT numerical values. POSIX defines the symbolic constants: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html The Python stat module is flawed in assuming it knows the numeric values (although by tradition, it is correct on all systems we care about).
Martin, what if C posix module (or whoever) would export the symbolic constants, and update "stat.py" to use those symbolic constants?. Do you think that would be an improvement?.
> Martin, what if C posix module (or whoever) would export the symbolic > constants, and update "stat.py" to use those symbolic constants?. > > Do you think that would be an improvement?. Improvement compared to what? The status quo? Certainly. Compared to a path where the stat module is deprecated? Certainly not.
Martin, I guess "stat" deprecation could require a few years and it would be an extra incompatibility burden between 2.7 and 3.x. Beside the symbolic constants, why would you see "stat" deprecated?.
Am 31.01.2011 12:46, schrieb Jesús Cea Avión: > > Jesús Cea Avión <jcea@jcea.es> added the comment: > > Martin, I guess "stat" deprecation could require a few years and it > would be an extra incompatibility burden between 2.7 and 3.x. > > Beside the symbolic constants, why would you see "stat" deprecated?. Because it doesn't provide any useful functionality, and confuses the users. The ST_* constants should have been deprecated long ago, since people really should use the st_ members of stat_result. In addition, stat_result should stop behaving like a tuple. The other constants and functions (S_*) are plainly non-portable. Apparently, contributor have the illusion that the values they put into the file are somehow standardized, when they are actually not. So having the module available makes people like you wonder how to best support it (see msg127107 and msg127110). Better remove it so that it doesn't confuse you anymore.
> Apparently, contributor have the illusion that the values they put > into the file are somehow standardized, when they are actually not. Are there actual bugs with that or is it merely a theoretical concern?
>> Apparently, contributor have the illusion that the values they put >> into the file are somehow standardized, when they are actually not. > > Are there actual bugs with that or is it merely a theoretical concern? Neither, nor (i.e. it's in the middle). I'm not aware of an actual problem, but I very much doubt that SF_SNAPSHOT really has the same meaning on all systems (IOW, chances are high that some system has allocated the same bit to mean something different). Looking in more detail: for the S_IFMT flags, OSX/Darwin/FreeBSD defines 0xe000 as S_IFWHT (whiteout), but Solaris defines it as S_IFPORT (event port).
After reviewing the code, I agree with Martin v. Löwis that "stat.py" is a joke. Neither can we trust the numerical constants (each OS is free to choose a different meaning) neither the code in "filemode()" can cope with modes with more than a bit active (like DOORS). I think all this code should be moved to a C module, able to access the underlining real constant definitions. What do you think?. Time to deprecate this and move the functionality into posix module, as suggested by Martin?.
The stat module (or whatever it is going to be in Python 3.4) is missing more checks than S_ISDOOR: # Solaris S_ISDOOR() S_ISPORT() # POSIX 1.b real-time extension S_ISMSG() S_ISSEM() S_ISSHM() # whiteout, translucent file systems S_ISWHT
I have done some research. The POSIX 1.b extensions aren't used on any supported system (neither Linux nor BSD, Solaris or AIX). random832@fastmail.us has compiled a list of even more file types: Heirloom toolchest "ls" supports: http://heirloom.cvs.sourceforge.net/viewvc/heirloom/heirloom/ls/ls.c?revision=1.9&view=markup http://heirloom.cvs.sourceforge.net/viewvc/heirloom/heirloom/ls/ls.1?revision=1.5&view=markup S_IFNWK HP-UX network special file S_IFNAM XENIX special named file S_INSEM XENIX semaphore subtype of IFNAM (looked up from s->rdev) S_INSHD XENIX shared data subtype of IFNAM " " " " Of these, GNU coreutils ls only supports doors and whiteouts. Chasing after a random hunch (something about AIX), I found these: http://cd.textfiles.com/transameritech2/EXTRAS/JOVE-4.6/ASK.C S_ISHIDDEN Hidden Directory [aix] S_ISCDF Context Dependent Files [hpux] S_ISNWK Network Special [hpux] http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg00084.html S_ISMPX AIX "MPX" file (multiplex device?) https://github.com/gagern/gnulib/blob/master/tests/test-sys_stat.c has a massive pile of macros with no comments S_ISCTG S_ISMPB S_ISMPX S_ISNAM S_ISNWK S_ISOFD S_ISOFL S_ISPORT http://lists.gnu.org/archive/html/bug-gnulib/2004-08/msg00017.html S_ISOFD Cray DMF (data migration facility): off line, with data S_ISOFL Cray DMF (data migration facility): off line, with no data S_ISCTG Contiguous (It's possible that these may not be file types) http://doiso.googlecode.com/svn/trunk/Source/mkisofs-1.12b5/include/statdefs.h S_ISMPC UNUSED multiplexed c S_ISNAM Named file (XENIX) S_ISMPB UNUSED multiplexed b S_ISCNT Contiguous file S_ISSHAD Solaris shadow inode http://www.opensource.apple.com/source/gnutar/gnutar-450/gnutar/lib/sys_stat_.h S_ISMPB /* V7 */ S_ISPORT /* Solaris 10 and up */ S_TYPEISSEM S_TYPEISSHM - macros to check the XENIX IFNAM types mentioned above S_TYPEISMQ S_TYPEISTMO
I have created a C implementation of the stat module for Python 3.4. It implements all current features, handling for DOOR, PORT, WHT and a fix for #17913. The first half of the file has lots of #ifndef checks. I'm not sure if they are really required. I guess they are required for the tarfile module on Windows.
Christian, could you post it as a mercurial diff for review? Also, it would maybe be better to rename this issue to "rewrite stat module in C". Shouldn't we also remove the Python version?
We've been *adding* python implementations for other modules, so I don't see why we would remove this one.
> We've been *adding* python implementations for other modules, so I > don't see why we would remove this one. Well, the one reason is that the C constants aren't accessible from Python code. Once the constants are there, the rest is so trivial that it doesn't really deserve a pure Python alternative, IMHO.
> We've been *adding* python implementations for other modules, so I don't see why we would remove this one. Because it's buggy, and cannot be implemented correctly in python.
"Cannot be implemented correctly in Python" is a darned good reason :)
If we use the C language, I would prefer to only expose what does really exist, and not add "fake" functions like:
+#ifndef S_ISDOOR
+# define S_ISDOOR(mode) 0
+#endif
So in:
+static PyMethodDef stat_methods[] = {
+ {"S_ISDIR", stat_S_ISDIR, METH_O, stat_S_ISDIR_doc},
+ {"S_ISCHR", stat_S_ISCHR, METH_O, stat_S_ISCHR_doc},
+ {"S_ISBLK", stat_S_ISBLK, METH_O, stat_S_ISBLK_doc},
+ {"S_ISREG", stat_S_ISREG, METH_O, stat_S_ISREG_doc},
+ {"S_ISFIFO", stat_S_ISFIFO, METH_O, stat_S_ISFIFO_doc},
+ {"S_ISLNK", stat_S_ISLNK, METH_O, stat_S_ISLNK_doc},
+ {"S_ISSOCK", stat_S_ISSOCK, METH_O, stat_S_ISSOCK_doc},
+ {"S_ISDOOR", stat_S_ISDOOR, METH_O, stat_S_ISDOOR_doc},
+ {"S_ISPORT", stat_S_ISPORT, METH_O, stat_S_ISPORT_doc},
+ {"S_ISWHT", stat_S_ISWHT, METH_O, stat_S_ISWHT_doc},
+ {"S_IMODE", stat_S_IMODE, METH_O, stat_S_IMODE_doc},
+ {"S_IFMT", stat_S_IFMT, METH_O, stat_S_IFMT_doc},
+ {"filemode", stat_filemode, METH_O, stat_filemode_doc},
+ {NULL, NULL} /* sentinel */
+};
I expect something similar to what is done in posixmodule.c:
static PyMethodDef stat_methods[] = {
{"S_ISDIR", stat_S_ISDIR, METH_O, stat_S_ISDIR_doc},
...
#ifdef S_ISDOOR
{"S_ISDOOR", stat_S_ISDOOR, METH_O, stat_S_ISDOOR_doc},
#endif
{"filemode", stat_filemode, METH_O, stat_filemode_doc},
{NULL, NULL} /* sentinel */
};
> I have created a C implementation of the stat module for Python 3.4. Oh, your C module is called "stat", as the "stat" module implemented in Python (Lib/stat.py). What is your plan? Replace completly the Python module with the C module? It may be nice to keep the Python module for compatibility with other Python implementations, and test both implementations.
AP has started to review my patch. I'm not yet sure how we should handle constants and functions when the platform doesn't provide them. For example Windows doesn't have any S_IS*() function like macros and doesn't provide a S_IFBLK constant. We have three possibilities here: 1) omit the constant / function (breaks b/w compatibility) 2) add constant with a value of 0 / function that returns false (may break b/w compatibility) 3) default to current value from Lib/stat.py I'm against 1) because it breaks backwards compatibility and makes the module harder to use. I like to follow 3) for all S_I*, SF_* and UF_* macros and functions that are currently provided by the stat module and 2) for new constants and functions such as S_ISDOOR().
New patch with extensive tests for the stat module. The tests are testing both the new C module and the old stat.py module.
The latest patch comes with more comments and documentation updates. The C implementation defines all existing constants to the hard coded values from stat.py if the platform doesn't provide them. The approach keeps maximum backward compatibility with the old stat module. I have also added the new constants and functions to the pure Python implementation for maximum compatibility with other Python implementation.
I still fail to understand why you just don't ditch the Python implementation.
New changeset f8ff61f44aca by Christian Heimes in branch '3.3': Add tests for untested features of the 'stat' module (part of issue #11016) http://hg.python.org/cpython/rev/f8ff61f44aca New changeset d15aee50e4a0 by Christian Heimes in branch 'default': Add tests for untested features of the 'stat' module (part of issue #11016) http://hg.python.org/cpython/rev/d15aee50e4a0
New patch People demand a _stat module in C and now they are getting a _stat module in C.
New changeset 420f70a22b9d by Christian Heimes in branch 'default': Issue #11016: Add C implementation of the stat module as _stat http://hg.python.org/cpython/rev/420f70a22b9d
buf[9] is not initialized in stat_filemode(). Use a shorter buffer (9 bytes) or set it to NUL. Le 22 juin 2013 21:05, "Roundup Robot" <report@bugs.python.org> a écrit : > > Roundup Robot added the comment: > > New changeset 420f70a22b9d by Christian Heimes in branch 'default': > Issue #11016: Add C implementation of the stat module as _stat > http://hg.python.org/cpython/rev/420f70a22b9d > > ---------- > > _______________________________________ > Python tracker <report@bugs.python.org> > <http://bugs.python.org/issue11016> > _______________________________________ >
All 10 chars are set: buf[0] = filetype(mode); fileperm(mode, &buf[1]); buf[0] is set by filetype(). fileperm() sets 9 chars in buf[1] to buf[9].
New changeset e5427b0b2bf7 by Victor Stinner in branch 'default': Issue #11016: Try to fix compilaton of the new _stat.c module on Windows http://hg.python.org/cpython/rev/e5427b0b2bf7
> fileperm() sets 9 chars in buf[1] to buf[9]. Ah ok, fine, I missed the "&buf[1]" hack.
> New changeset e5427b0b2bf7 by Victor Stinner in branch 'default': > Issue #11016: Try to fix compilaton of the new _stat.c module on Windows > http://hg.python.org/cpython/rev/e5427b0b2bf7 @Christian: Can you please review this commit? By the way, mode_t is also defined in import.c: #ifdef MS_WINDOWS /* for stat.st_mode */ typedef unsigned short mode_t; /* for _mkdir */ #include <direct.h> #endif And stat_filemode() should detect integer overflow. mode_t is a 32-bit unsigned integer on Linux, and now a 16-bit integer on Windows, whereas stat_filemode() uses an unsigned long (which 32 bit on Windows, and 32 or 64 bits on Linux).
> And stat_filemode() should detect integer overflow. Attached stat_mode_overflow.patch should fix this issue. (I would also suggest to inline fileperm() into stat_filemode(), or pass buf instead of &buf[1]. But you may not agree, as you want :-))
My changeset e5427b0b2bf7 is not enough: "import _stat" still fail on Windows. See for example: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/2164/steps/test/logs/stdio ====================================================================== ERROR: test_directory (test.test_stat.TestFilemodeCStat) ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_stat.py", line 128, in test_directory st_mode, modestr = self.get_mode() File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_stat.py", line 67, in get_mode modestr = self.statmod.filemode(st_mode) AttributeError: 'NoneType' object has no attribute 'filemode' ====================================================================== ERROR: test_mode (test.test_stat.TestFilemodeCStat) ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_stat.py", line 119, in test_mode st_mode, modestr = self.get_mode() File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_stat.py", line 67, in get_mode modestr = self.statmod.filemode(st_mode) AttributeError: 'NoneType' object has no attribute 'filemode' ====================================================================== ERROR: test_module_attributes (test.test_stat.TestFilemodeCStat) ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_stat.py", line 169, in test_module_attributes modvalue = getattr(self.statmod, key) AttributeError: 'NoneType' object has no attribute 'ST_DEV'
test_stat.test_devices() fail on Solaris: it looks like os.devnull is a symlink. You should probably use os.stat() instead of os.lstat() for this specific test. http://buildbot.python.org/all/builders/SPARC%20Solaris%2010%20%28cc%2C%2032b%29%20%5BSB%5D%203.x/builds/708/steps/test/logs/stdio ====================================================================== FAIL: test_devices (test.test_stat.TestFilemodeCStat) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/cpython/buildslave/cc-32/3.x.snakebite-sol10-sparc-cc-32/build/Lib/test/test_stat.py", line 157, in test_devices self.assertEqual(modestr[0], 'c') AssertionError: 'l' != 'c' - l + c
I have addressed the Windows build issue in http://hg.python.org/cpython/rev/838f04e5a690 and the failing test on Solaris in http://hg.python.org/cpython/rev/6c23ca1982b3 (also 2.7 and default).
New changeset 44c8a9d80595 by Victor Stinner in branch 'default': Issue #11016: Detect integer conversion on conversion from Python int to C mode_t http://hg.python.org/cpython/rev/44c8a9d80595
New changeset 75bc0ae02bcd by Christian Heimes in branch 'default': Issue #11016: Don't define macros and constants that are already set by pyport.h http://hg.python.org/cpython/rev/75bc0ae02bcd
Can we re-close this issue? Or is there still something to do?
Let's close it.
resolution: fixed
messages: + msg192168
stage: patch review -> resolved
messages: + msg191692
messages: + msg191596
messages:
+ msg191492
title: stat module in C -> Re-implementation of the stat module in C
messages: + msg191486
title: Add S_ISDOOR to the stat module -> stat module in C
messages: + msg188610
keywords: + patch
messages: + msg188547
messages: + msg188526
messages:
+ msg188519
stage: patch review
messages: + msg188300
messages: + msg127673
messages: + msg127672
messages: + msg127671
messages: + msg127592
messages: + msg127579
messages: + msg127567
messages: + msg127138
messages: + msg127137
messages: + msg127118
messages: + msg127117
messages: + msg127114
messages: + msg127111
messages: + msg127110
messages: + msg127108
messages: + msg127107
messages: + msg127105
messages: + msg127103
nosy: loewis, jcea, pitrou, movement