Repository navigation
Add some posix functions #55021
Description
| BPO | 10812 |
|---|---|
| Nosy | @loewis, @birkenfeld, @gpshead, @ronaldoussoren, @abalkin, @pitrou, @giampaolo, @ned-deily |
| Files |
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:
assignee = None
closed_at = <Date 2011-03-17.18:24:48.122>
created_at = <Date 2011-01-03.12:45:50.277>
labels = ['extension-modules', 'type-feature']
title = 'Add some posix functions'
updated_at = <Date 2012-07-31.17:55:28.929>
user = 'https://bugs.python.org/rosslagerwall'bugs.python.org fields:
activity = <Date 2012-07-31.17:55:28.929>
actor = 'ronaldoussoren'
assignee = 'rosslagerwall'
closed = True
closed_date = <Date 2011-03-17.18:24:48.122>
closer = 'rosslagerwall'
components = ['Extension Modules']
creation = <Date 2011-01-03.12:45:50.277>
creator = 'rosslagerwall'
dependencies = []
files = ['20239', '20273', '20292', '20305', '20311', '20461', '21250']
hgrepos = []
issue_num = 10812
keywords = ['patch']
message_count = 26.0
messages = ['125162', '125174', '125183', '125187', '125414', '125454', '125555', '125604', '125634', '125690', '125729', '125730', '125749', '126598', '131088', '131090', '131111', '131168', '131169', '131275', '131282', '131398', '167012', '167015', '167016', '167021']
nosy_count = 11.0
nosy_names = ['loewis', 'georg.brandl', 'gregory.p.smith', 'ronaldoussoren', 'belopolsky', 'pitrou', 'giampaolo.rodola', 'ned.deily', 'Arfrever', 'rosslagerwall', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue10812'
versions = ['Python 3.3']
Activity
rosslagerwall commented on Jan 3, 2011
Here's a patch that adds a bunch of posix functions that are missing from the posix module. Includes tests & documentation.
Tested on Linux & FreeBSD.
Specifically:
futimes
lutimes
futimens
fexecve
gethostid
sethostname
waitid
lockf
readv
pread
writev
pwrite
truncate
posix_fallocate
posix_fadvise
sync
pitrou commented on Jan 3, 2011
First couple comments:
pitrou commented on Jan 3, 2011
For the record, I get the following failures under OpenSolaris:
======================================================================
ERROR: test_lutimes (test.test_posix.PosixTester)
----------------------------------------------------------------------
======================================================================
ERROR: test_posix_fallocate (test.test_posix.PosixTester)
----------------------------------------------------------------------
pitrou commented on Jan 3, 2011
According to the posix_fallocate() man page under OpenSolaris:
EINVAL The len argument is less than or equal to zero, or the offset argument is less than zero, or the underlying file system does not support this operation.I would go for the third (last) interpretation: the filesystem (ZFS here) doesn't support it.
rosslagerwall commented on Jan 5, 2011
This patch:
Fixes test_lutimes(),
Ignores a posix_fallocate() failure on Solaris due to ZFS,
Changes readv() & writev() signatures such that they take sequences instead of tuples,
Changes readv() so that it takes a number of writable buffers, fills them and returns the total number of bytes read.
abalkin commented on Jan 5, 2011
The patch contains a lot of repeated boilerplate code. I wonder if some of it can be factored out and reused. For example iov buffer allocation code appears to be identical in writev and readv.
rosslagerwall commented on Jan 6, 2011
This new patch reuses iov allocation code between readv & writev.
It reuses code between exec, execve & fexecve.
It reuses code for parsing off_t types.
I've tried where possible to reuse code but I think though that the code seems pretty standard for the posix module. Each function pretty much parses args, calls posix function, checks for error and returns result.
loewis commented on Jan 6, 2011
Patch looks mostly good. Some comments:
wrapper should not use strlen
module and supports Windows, and sethostname is in the POSIX
module. It would be useful to have a gethostname in the POSIX
module also which is a) POSIX only and b) supports embedded
NUL bytes.
are returning from siginfo_t are actually filled.
rosslagerwall commented on Jan 7, 2011
According to the spec for gethostname(), the hostname that it returns is null-terminated so it won't support embedded NUL bytes. Should we still add it anyway?
rosslagerwall commented on Jan 7, 2011
Thanks for the comments.
I implemented sethostid() - its not actually in the posix spec, but linux/*bsd
have it although the signature for FreeBSD is a bit different.
I implemented gethostname(). Both get/sethostname() now use FSDefault encoding.
gethostname() is null-terminated so it doesn't have to deal with embedded NULs.
sethostname() works with embedded NULs.
Since id_t can contain a pid_t, I now parse id_t with _Py_PARSE_PID.
waitid() also now returns a structsequence of the correct fields.
According to the spec, gethostid does not set errno - it now checks anyway.
Instead of PARSE_OFF_T, I use a O& parser.
loewis commented on Jan 7, 2011
Oops, I misread the spec. No, gethostname should then not be duplicated.
Putting sethostname into the socket module might be worth considering,
though; I find the proposed split confusing.
loewis commented on Jan 7, 2011
Sorry, I misread that also. Leaving the check is fine; reverting it to
the previous code would be fine as well.
rosslagerwall commented on Jan 8, 2011
This patch takes out sethostname() and gethostname(). I'll open up a new issue to add sethostname() to the socket module.
1 remaining item
gpshead commented on Mar 16, 2011
I added some comments on the review for 10812_v5.patch. not sure why v6 doesn't have a review link. Overall, very nice after addressing the few comments I had.
btw, can you sync this up with the hg tip (3.3) now while addressing the above?
I'm excited to see readv/writev/fadvise added. Extremely useful.
rosslagerwall commented on Mar 16, 2011
PyParse_off_t was already added when sendfile() was added as was the iovec_setup stuff.
I'll upload a patch soon which updates it to take this and the other comments into account.
loewis commented on Mar 16, 2011
v6 was created against an unknown mercurial repository (most likely code.python.org). It actually doesn't apply cleanly anymore so it would be best to regenerate it it (or go on with any v7 that may result out of the review).
rosslagerwall commented on Mar 16, 2011
This new patch, updated against the tip for 3.3:
Shares the iov_setup and iov_cleanup code, py_parse_off_t with sendfile().
Removes gethostid and sethostid since they're deprecated.
I think I was correct with referring to utime() since I was referring to the python function, os.utime().
rosslagerwall commented on Mar 16, 2011
Forgot to attach the patch
python-dev commented on Mar 17, 2011
New changeset 525320df8afb by Ross Lagerwall in branch 'default':
Issue bpo-10812: Add some extra posix functions to the os module.
http://hg.python.org/cpython/rev/525320df8afb
python-dev commented on Mar 17, 2011
New changeset 8945e087a5a6 by Ross Lagerwall in branch 'default':
Issue bpo-10812: Revert os.lseek change.
http://hg.python.org/cpython/rev/8945e087a5a6
python-dev commented on Mar 19, 2011
New changeset 96e09d039433 by Ross Lagerwall in branch 'default':
Fix refleak introduced by bpo-10812.
http://hg.python.org/cpython/rev/96e09d039433
ronaldoussoren commented on Jul 31, 2012
msg126598 says that waitid is broken on MacOSX. In what way it is broken?
The code is currently disabled for OSX without any indication why this is so, that makes it hard to know how to test if new OSX releases (like the just released OSX 10.8) fix waitid.
ronaldoussoren commented on Jul 31, 2012
To elaborate: when I remove the "&& !defined(APPLE)" tests from posixmodule.c test_posix passes on OSX 10.8, including test_waitid.
I have no idea if this means that waitid is no longer "broken" in OSX 10.8 or that test_posix doesn't trigger the issue.
rosslagerwall commented on Jul 31, 2012
I can't actually remember why I disabled waitid for OS X - that was message was rather a long time ago :-(
Unfortunately, I don't currently have access to an OS X machine to test it.
A google search shows the following comment in the v8 javascript engine:
// We're disabling usage of waitid in Mac OS X because it doens't work for us:
// a parent process hangs on waiting while a child process is already a zombie.
// See http://code.google.com/p/v8/issues/detail?id=401.
Do you have access to test an older release like Leopard?
ronaldoussoren commented on Jul 31, 2012
I have VMs with earlier versions of OSX (at least 10.5 and 10.6). I cannot run those right now because they use enough resources to interfere with other activities. I'll try to test later this week.
BTW. I'd prefer to make waitid available on OSX unless it is totally non-functional. I'm not convinced that the Google problem is a good enough reason to avoid exposing the function. If it is disabled there should be a clear description of why that is, preferable in the form of a (configure) test.