classification
Title: Add some posix functions
Type: enhancement Stage: committed/rejected
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rosslagerwall Nosy List: Arfrever, belopolsky, georg.brandl, giampaolo.rodola, gregory.p.smith, loewis, ned.deily, pitrou, python-dev, ronaldoussoren, rosslagerwall
Priority: normal Keywords: patch

Created on 2011-01-03 12:45 by rosslagerwall, last changed 2012-07-31 17:55 by ronaldoussoren. This issue is now closed.

Files
File name Uploaded Description Edit
mpos.patch rosslagerwall, 2011-01-03 12:45 Patch + test for issue review
10812_v2.patch rosslagerwall, 2011-01-05 06:16 Updated patch review
10812_v3.patch rosslagerwall, 2011-01-06 14:33 Updated patch review
10812_v4.patch rosslagerwall, 2011-01-07 19:19 Updated patch, added sethostid, gethostname review
10812_v5.patch rosslagerwall, 2011-01-08 04:31 Updated patch, removed get/sethostname review
10812_v6.patch rosslagerwall, 2011-01-20 11:35 Fixes for OS X
10812_v7.patch rosslagerwall, 2011-03-16 20:20
Messages (26)
msg125162 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-03 12:45
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
msg125174 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 14:14
First couple comments:
- you don't have to modify Misc/NEWS yourself; it will probably make patch maintenance easier
- it would seem more natural for readv() to take a sequence of writable buffers (such as bytearrays) instead; I don't think the current signature is very useful
- readv() and writev() should support both lists and tuples, at the minimum (perhaps arbitrary iterables if you like to spend more time on it :-)): see the PySequence* API
msg125183 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 15:42
For the record, I get the following failures under OpenSolaris:

======================================================================
ERROR: test_lutimes (test.test_posix.PosixTester)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/antoine/py3k/cc/Lib/test/test_posix.py", line 265, in test_lutimes
    posix.lutimes(support.TESTFN, None)
AttributeError: 'module' object has no attribute 'lutimes'

======================================================================
ERROR: test_posix_fallocate (test.test_posix.PosixTester)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/antoine/py3k/cc/Lib/test/test_posix.py", line 236, in test_posix_fallocate
    posix.posix_fallocate(fd, 0, 10)
OSError: [Errno 22] Invalid argument
msg125187 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 15:58
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.
msg125414 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-05 06:16
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.
msg125454 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-01-05 19:01
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.
msg125555 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-06 14:33
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.
msg125604 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-06 23:30
Patch looks mostly good. Some comments:

- sethostname supports names with embedded null bytes, so
  wrapper should not use strlen
- it's a bit asymmetric that gethostname is in the socket
  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.
- I think get/sethostname should use the FSDefault encoding.
- gethostid should check for POSIX errors.
- it checks for sethostid but then doesn't implement it.
- parsing id_t with "l" is incorrect, methinks
- siginfo_t should map to a structsequence
- according to my copy of waitid(2), not all of the fields you
  are returning from siginfo_t are actually filled.
- instead of PARSE_OFF_T, I rather recommend to use an O& parser.
msg125634 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-07 09:19
>  it's a bit asymmetric that gethostname is in the socket
>  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.

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?
msg125690 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-07 19:19
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.
msg125729 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-07 23:40
> 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?

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.
msg125730 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-07 23:43
> According to the spec, gethostid does not set errno - it now checks anyway.

Sorry, I misread that also. Leaving the check is fine; reverting it to
the previous code would be fine as well.
msg125749 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-08 04:31
This patch takes out sethostname() and gethostname(). I'll open up a new issue to add sethostname() to the socket module.
msg126598 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-20 11:35
A few small fixes for OS X:
It has no return value for sethostid() and sets different errno if permission denied,
waitid() is broken - so its disabled,
the timeval struct used in futimes and lutimes is defined slightly differently.
msg131088 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2011-03-16 05:19
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.
msg131090 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-03-16 06:19
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.
msg131111 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-03-16 13:30
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).
msg131168 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-03-16 20:20
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().
msg131169 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-03-16 20:20
Forgot to attach the patch
msg131275 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-17 18:21
New changeset 525320df8afb by Ross Lagerwall in branch 'default':
Issue #10812: Add some extra posix functions to the os module.
http://hg.python.org/cpython/rev/525320df8afb
msg131282 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-17 19:56
New changeset 8945e087a5a6 by Ross Lagerwall in branch 'default':
Issue #10812: Revert os.lseek change.
http://hg.python.org/cpython/rev/8945e087a5a6
msg131398 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-19 07:19
New changeset 96e09d039433 by Ross Lagerwall in branch 'default':
Fix refleak introduced by #10812.
http://hg.python.org/cpython/rev/96e09d039433
msg167012 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2012-07-31 16:08
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.
msg167015 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2012-07-31 16:29
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.
msg167016 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2012-07-31 16:35
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?
msg167021 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2012-07-31 17:55
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.
History
Date User Action Args
2012-07-31 17:55:28ronaldoussorensetmessages: + msg167021
2012-07-31 16:35:14rosslagerwallsetmessages: + msg167016
2012-07-31 16:29:21ronaldoussorensetmessages: + msg167015
2012-07-31 16:08:18ronaldoussorensetnosy: + ronaldoussoren
messages: + msg167012
2011-03-19 07:19:59python-devsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall, python-dev
messages: + msg131398
2011-03-17 19:56:22python-devsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall, python-dev
messages: + msg131282
2011-03-17 18:24:48rosslagerwallsetstatus: open -> closed
assignee: rosslagerwall
resolution: accepted
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall, python-dev
stage: patch review -> committed/rejected
2011-03-17 18:21:13python-devsetnosy: + python-dev
messages: + msg131275
2011-03-16 20:20:46rosslagerwallsetfiles: + 10812_v7.patch
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall
messages: + msg131169
2011-03-16 20:20:15rosslagerwallsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall
messages: + msg131168
2011-03-16 13:30:53loewissetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall
messages: + msg131111
2011-03-16 06:19:27rosslagerwallsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall
messages: + msg131090
2011-03-16 05:19:38gregory.p.smithsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, ned.deily, Arfrever, rosslagerwall
messages: + msg131088
stage: patch review
2011-03-11 19:05:05Arfreversetnosy: + Arfrever
2011-01-20 20:03:20ned.deilysetnosy: + ned.deily
2011-01-20 11:35:09rosslagerwallsetfiles: + 10812_v6.patch
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg126598
2011-01-08 04:31:06rosslagerwallsetfiles: + 10812_v5.patch
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125749
2011-01-07 23:43:25loewissetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125730
2011-01-07 23:40:25loewissetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125729
2011-01-07 19:19:56rosslagerwallsetfiles: + 10812_v4.patch
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125690
2011-01-07 09:19:14rosslagerwallsetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125634
2011-01-06 23:30:05loewissetnosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125604
2011-01-06 14:33:23rosslagerwallsetfiles: + 10812_v3.patch
nosy: loewis, georg.brandl, gregory.p.smith, belopolsky, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125555
2011-01-05 19:01:15belopolskysetnosy: + belopolsky
messages: + msg125454
2011-01-05 06:16:54rosslagerwallsetfiles: + 10812_v2.patch
nosy: loewis, georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125414
2011-01-03 15:58:49pitrousetnosy: loewis, georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125187
2011-01-03 15:42:54pitrousetnosy: loewis, georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125183
2011-01-03 14:14:09pitrousetnosy: loewis, georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125174
2011-01-03 12:45:50rosslagerwallcreate