Add some posix functions

@loewis

BPO	10812
Nosy	@loewis, @birkenfeld, @gpshead, @ronaldoussoren, @abalkin, @pitrou, @giampaolo, @ned-deily
Files	mpos.patch: Patch + test for issue 10812_v2.patch: Updated patch 10812_v3.patch: Updated patch 10812_v4.patch: Updated patch, added sethostid, gethostname 10812_v5.patch: Updated patch, removed get/sethostname 10812_v6.patch: Fixes for OS X 10812_v7.patch

^{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']

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

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

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

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.

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.

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.

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.

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.

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?

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.

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.

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.

This patch takes out sethostname() and gethostname(). I'll open up a new issue to add sethostname() to the socket module.

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.

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.

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).

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().

Forgot to attach the patch

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

New changeset 8945e087a5a6 by Ross Lagerwall in branch 'default':
Issue bpo-10812: Revert os.lseek change.
http://hg.python.org/cpython/rev/8945e087a5a6

New changeset 96e09d039433 by Ross Lagerwall in branch 'default':
Fix refleak introduced by bpo-10812.
http://hg.python.org/cpython/rev/96e09d039433

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.

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.

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?

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.

Navigation Menu

Repository navigation

Add some posix functions #55021

1 remaining item

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Add some posix functions #55021

Description

Activity

rosslagerwall commented on Jan 3, 2011

pitrou commented on Jan 3, 2011

pitrou commented on Jan 3, 2011

pitrou commented on Jan 3, 2011

rosslagerwall commented on Jan 5, 2011

abalkin commented on Jan 5, 2011

rosslagerwall commented on Jan 6, 2011

loewis commented on Jan 6, 2011

rosslagerwall commented on Jan 7, 2011

rosslagerwall commented on Jan 7, 2011

loewis commented on Jan 7, 2011

loewis commented on Jan 7, 2011

rosslagerwall commented on Jan 8, 2011

1 remaining item

gpshead commented on Mar 16, 2011

rosslagerwall commented on Mar 16, 2011

loewis commented on Mar 16, 2011

rosslagerwall commented on Mar 16, 2011

rosslagerwall commented on Mar 16, 2011

python-dev commented on Mar 17, 2011

python-dev commented on Mar 17, 2011

python-dev commented on Mar 19, 2011

ronaldoussoren commented on Jul 31, 2012

ronaldoussoren commented on Jul 31, 2012

rosslagerwall commented on Jul 31, 2012

ronaldoussoren commented on Jul 31, 2012

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions