homepage

Antoine, i made you noisy because you took care about Issue11323.
I'm at r88671 (271057c7c6f3).
I'm opening this because the buildbot URL (thanks) doesn't show
anything about sendfile(2).
12:47 ~/arena/code.extern.repos/py3k.hg $ ./python.exe -Wd -m test.test_os -r -w -v
...
test_headers (__main__.TestSendfile) ... FAIL
test_invalid_offset (__main__.TestSendfile) ... ok
test_offset_overflow (__main__.TestSendfile) ... ok
test_send_at_certain_offset (__main__.TestSendfile) ... ok
test_send_whole_file (__main__.TestSendfile) ... ok
test_trailers (__main__.TestSendfile) ... /Users/steffen/arena/code.extern.repos/py3k.hg/Lib/unittest/case.py:387: ResourceWarning: unclosed file <_io.BufferedReader name='@test_28253_tmp2'>
 function()
ok
test_set_get_priority (__main__.ProgramPriorityTests) ... ok
======================================================================
FAIL: test_headers (__main__.TestSendfile)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/test_os.py", line 1517, in test_headers
 self.assertEqual(total_sent, len(expected_data))
AssertionError: 163840 != 164352
----------------------------------------------------------------------
Ran 92 tests in 0.447s
FAILED (failures=1, skipped=10)
Traceback (most recent call last):
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/runpy.py", line 160, in _run_module_as_main
 "__main__", fname, loader, pkg_name)
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/runpy.py", line 73, in _run_code
 exec(code, run_globals)
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/test_os.py", line 1573, in <module>
 test_main()
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/test_os.py", line 1569, in test_main
 ProgramPriorityTests,
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/support.py", line 1187, in run_unittest
 _run_suite(suite)
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/support.py", line 1170, in _run_suite
 raise TestFailed(err)
test.support.TestFailed: Traceback (most recent call last):
 File "/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/test/test_os.py", line 1517, in test_headers
 self.assertEqual(total_send, len(expected_data))
AssertionError: 163840 != 164352

It's about the headers[]:
 def test_headers(self):
 total_sent = 0
 sent = os.sendfile(self.sockno, self.fileno, 0, 4096,
 headers=[b"x" * 512])
 total_sent += sent
 print("HEADER SENT:", sent)
prints
test_headers (__main__.TestSendfile) ... HEADER SENT: 4096
Well. 'man 2 sendfile' tells me it is supporting headers and
trailers since Darwin 9.0 aka Mac OS X 10.5.

posixmodule.c:
	iov_setup():
		IOV[i]: buffer:0x10152b8f0, len:512
		iov_setup() leave ok
	before sendfile: sf=0x7fff5fbfb410, sf.headers=0x10036c780
	sf.headers[0].iov_base=0x10152d950, .iov_len=512
	after sendfile: sbytes=4096
Maybe Mac OS X does not support headers (i'm on Mac OS X 10.6)?

Did you know that 'sbytes' is not adjusted to match possibly 
existent headers and trailers in posixmodule.c? 
This however is required according to Mac OS X 'man 2 sendfile'.
I'll attach a simple, naive patch for a very intelligent and 
outstanding operating system.

Likely explanation is http://bugs.python.org/issue10882#msg129555.

Before i'm going down, here is the paragraph from 'man 2 sendfile':
 When a header or trailer is specified the value of len returned will
 include the size of header or trailer sent. The user should provide suf-
 ficiently large value of len as argument including the size of header or
 trailer, otherwise only part of file data will be sent following the header.

(But see msg12970. And the test runs fine with my patch applied.)
On Mon, Feb 28, 2011 at 02:17:46PM +0000, Antoine Pitrou wrote:
> 
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> Likely explanation is http://bugs.python.org/issue10882#msg129555.
> 
> ----------
> nosy: +giampaolo.rodola
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue11351>
> _______________________________________

(Should have been msg129706, sorry.)
P.S.: yes, you're right.
But now: issue 10882 is on adding os.sendfile(), not on fixing 
errors on os.sendfile(). ;-)

Your patch is not cleaned up. There are strange things like:
+ ret ^= ret;
(while would you xor an off_t?)
and
+ do { off_t x = (*buf)[i].len;
+ (*iov)[i].iov_len = x;
+ ret += x;
+ } while (0);

Will this do? Otherwise feel free to adjust the patch the way 
Python needs it, i'll learn from that, then, for possible future 
things. (I need to go down now.)
On Mon, Feb 28, 2011 at 02:37:16PM +0000, Antoine Pitrou wrote:
> 
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> Your patch is not cleaned up. There are strange things like:
> 
> + ret ^= ret;
> 
> (while would you xor an off_t?)
> 
> and
> 
> + do { off_t x = (*buf)[i].len;
> + (*iov)[i].iov_len = x;
> + ret += x;
> + } while (0);

This version uses Py_ssize_t for the lengths and seems to be more 
Python-style beside that, too. 
It compiles clean and the test is ok.

I'm trashing your mailbox, but 
http://bugs.python.org/file20949/issue11351-2.patch should do 
'total+=blen' not 'total+=len'.
Anyway i attach yet a third patch which does some code cleanup in 
general and is a bit more of my very own coding style, because 
i have seen that gotos seem to be possible in Python, and because 
i was unable to enjoy the code (and this is beautiful C!) before. 
It also follows PEP guidelines more than before.
This compiles clean and runs the test even if i
 hg revert && hg patch
(I'm a bit messy)

Well the third patch has a lot of spurious style changes that make it harder to spot the actual changes.

Aaaaaah, unfortunately - what a shame, this beautiful language ... 
But do i need to re-post the real (ouch!) issue11351-2.patch, i.e. with 
'total+=blen' instead of 'total+=len'? 
Or is it sufficient now with these comments (for Giampaolo')?
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> Well the third patch has a lot of spurious style changes that make it harder to spot the actual changes.

Maybe Giampaolo' should at least replace these two 
PyArg_ParseTupleAndKeywords() near (unpatched) line 5942, 
because doing it like this is messes up (at least) vim(1) group 
detection, resulting in a lot of bold red mis-brace warnings 
further on?

Please try to provide a patch which fixes (as in "makes the test pass") this specific issue only. As for other changes such as the code restyling you can open a separate ticket with another patch.

> Maybe Giampaolo' should at least replace these two 
> PyArg_ParseTupleAndKeywords() near (unpatched) line 5942, 
> because doing it like this is messes up (at least) vim(1) group 
> detection, resulting in a lot of bold red mis-brace warnings 
> further on?
Problem is, each developer has his/her own preferences and editor
settings, so if we start reformatting with each commit, the history of a
piece of code becomes very difficult to read.
I would simply suggest that you disable these vim settings, or at least
tweak them so that they don't apply automatically when you edit existing
code.

So this is fixed -2.patch supplemented only with 
PyArg_ParseTupleAndKeywords() fixed in a way that re-enables 
correct syntax highlighting.

(GMail imposes a five minute or so delay when used over SMTP, so 
the timeline sometimes looks unfortunate.)
So, finally, -2.patch only fixed with 'blen' for 'len'.
Compiles and test ok.
> Giampaolo Rodola' <g.rodola@gmail.com> added the comment:
> 
> Please try to provide a patch which fixes (as in "makes the test pass") this specific issue only. As for other changes such as the code restyling you can open a separate ticket with another patch.

But, dear Antoine, i don't know *any* editor that is capable to 
handle the following code correctly:
#ifdef __APPLE__
 if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iiO&O&|OOi:sendfile",
 keywords, &out, &in, _parse_off_t, &offset, _parse_off_t, &sbytes,
#else
 if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iiO&n|OOi:sendfile",
 keywords, &out, &in, _parse_off_t, &offset, &len,
#endif
 &headers, &trailers, &flags))
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> Problem is, each developer has his/her own preferences and editor
> settings, so if we start reformatting with each commit, the history of a
> piece of code becomes very difficult to read.
> 
> I would simply suggest that you disable these vim settings, or at least
> tweak them so that they don't apply automatically when you edit existing
> code.

> But, dear Antoine, i don't know *any* editor that is capable to 
> handle the following code correctly:
> 
> #ifdef __APPLE__
> if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iiO&O&|OOi:sendfile",
> keywords, &out, &in, _parse_off_t, &offset, _parse_off_t, &sbytes,
> #else
> if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iiO&n|OOi:sendfile",
> keywords, &out, &in, _parse_off_t, &offset, &len,
> #endif
> &headers, &trailers, &flags))
What do you mean with "handling"? If you are asking your editor to
understand the C language, then it might indeed have a problem.
If you are asking your editor to simply edit test, it should work fine.

I agree with that.
> Antoine Pitrou <pitrou@free.fr> added the comment:
> If you are asking your editor to simply edit test, it should work fine.

> Giampaolo Rodola' <g.rodola@gmail.com> added the comment:
> 
> Please try to provide a patch which fixes (as in "makes the test pass") this specific issue only. As for other changes such as the code restyling you can open a separate ticket with another patch.
Forza, Giampaolo. :)
If you never patch file20960 in, the buildbots will fail 
for the time being.
(And they do have to run Mac OS X already...)

issue11351-5.patch no longer applies cleanly and I don't have an OSX box to test against at the moment.
Could you please update your patch? If you tell me it's gonna fix the test I'm going to commit it assuming everything is fine.

> Giampaolo Rodola' <g.rodola@gmail.com> added the comment:
> 
> issue11351-5.patch no longer applies cleanly and I don't have an OSX box to test against at the moment.
> Could you please update your patch? If you tell me it's gonna fix the test I'm going to commit it assuming everything is fine.
I'm at e79364a6bed8/[svn r88726].
issue11351-5.patch applies and the test is ok, at least in respect to sendfile:
Using random seed 1578968
[1/1] test_os
/Users/steffen/arena/code.extern.repos/py3k.hg/Lib/unittest/case.py:387: ResourceWarning: unclosed file <_io.BufferedReader name='@test_59563_tmp2'>
 function()
1 test OK.

It's ok, but for the statistics:
16:59 ~/arena/code.extern.repos/py3k.hg $ hg identify
e79364a6bed8 (py3k) tip
16:59 ~/arena/code.extern.repos/py3k.hg $ hg slog -r tip
Changeset: 10021:e79364a6bed8
branch: py3k
tag: tip
user: giampaolo.rodola
date: Thu Mar 03 15:10:58 2011 +0100
summary: [svn r88726] fix attribute error
16:59 ~/arena/code.extern.repos/py3k.hg $ sh .hg/steffen/buildit.sh
./configure --prefix=$HOME/usr/opt/py3k MACOSX_DEPLOYMENT_TARGET=10.6
make all
17:04 ~/arena/code.extern.repos/py3k.hg $ hg patch --no-commit --strip 1 .hg/steffen/issue11351-5.patch 
applying .hg/steffen/issue11351-5.patch
17:04 ~/arena/code.extern.repos/py3k.hg $ make all
/usr/bin/gcc -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall 
-Wstrict-prototypes -O2 -O2 -I. -IInclude -I./Include 
-DPy_BUILD_CORE -c ./Modules/posixmodule.c -o 
Modules/posixmodule.o
/usr/bin/gcc
...
17:05 ~/arena/code.extern.repos/py3k.hg $ ./python.exe -E -Wd -m test.test_os -r -w -uall
test_access (__main__.FileTests) ... ok
test_closerange (__main__.FileTests) ... ok
test_read (__main__.FileTests) ... ok
test_rename (__main__.FileTests) ... ok
test_write (__main__.FileTests) ... ok
test_stat_attributes (__main__.StatAttributeTests) ... ok
test_stat_attributes_bytes (__main__.StatAttributeTests) ... ok
test_statvfs_attributes (__main__.StatAttributeTests) ... ok
test_utime_dir (__main__.StatAttributeTests) ... ok
test___repr__ (__main__.EnvironTests)
Check that the repr() of os.environ looks like environ({...}). ... ok
test_bool (__main__.EnvironTests) ... ok
test_constructor (__main__.EnvironTests) ... ok
test_environb (__main__.EnvironTests) ... ok
test_get (__main__.EnvironTests) ... ok
test_get_exec_path (__main__.EnvironTests) ... ok
test_getitem (__main__.EnvironTests) ... ok
test_items (__main__.EnvironTests) ... ok
test_keys (__main__.EnvironTests) ... ok
test_keyvalue_types (__main__.EnvironTests) ... ok
test_len (__main__.EnvironTests) ... ok
test_os_popen_iter (__main__.EnvironTests) ... ok
test_pop (__main__.EnvironTests) ... ok
test_popitem (__main__.EnvironTests) ... ok
test_read (__main__.EnvironTests) ... ok
test_setdefault (__main__.EnvironTests) ... ok
test_update (__main__.EnvironTests) ... ok
test_update2 (__main__.EnvironTests) ... ok
test_values (__main__.EnvironTests) ... ok
test_write (__main__.EnvironTests) ... ok
test_traversal (__main__.WalkTests) ... ok
test_exist_ok_existing_directory (__main__.MakedirTests) ... ok
test_exist_ok_existing_regular_file (__main__.MakedirTests) ... ok
test_makedir (__main__.MakedirTests) ... ok
test_devnull (__main__.DevNullTests) ... ok
test_urandom (__main__.URandomTests) ... ok
test_execvpe_with_bad_arglist (__main__.ExecTests) ... ok
test_execvpe_with_bad_program (__main__.ExecTests) ... ok
test_internal_execvpe_str (__main__.ExecTests) ... ok
test_closerange (__main__.TestInvalidFD) ... ok
test_dup (__main__.TestInvalidFD) ... ok
test_dup2 (__main__.TestInvalidFD) ... ok
test_fchdir (__main__.TestInvalidFD) ... ok
test_fchmod (__main__.TestInvalidFD) ... ok
test_fchown (__main__.TestInvalidFD) ... ok
test_fdatasync (__main__.TestInvalidFD) ... ok
test_fdopen (__main__.TestInvalidFD) ... ok
test_fpathconf (__main__.TestInvalidFD) ... ok
test_fstat (__main__.TestInvalidFD) ... ok
test_fstatvfs (__main__.TestInvalidFD) ... ok
test_fsync (__main__.TestInvalidFD) ... ok
test_ftruncate (__main__.TestInvalidFD) ... ok
test_isatty (__main__.TestInvalidFD) ... ok
test_lseek (__main__.TestInvalidFD) ... ok
test_read (__main__.TestInvalidFD) ... ok
test_tcgetpgrp (__main__.TestInvalidFD) ... ok
test_tcsetpgrpt (__main__.TestInvalidFD) ... ok
test_ttyname (__main__.TestInvalidFD) ... ok
test_write (__main__.TestInvalidFD) ... ok
test_setegid (__main__.PosixUidGidTests) ... ok
test_seteuid (__main__.PosixUidGidTests) ... ok
test_setgid (__main__.PosixUidGidTests) ... ok
test_setregid (__main__.PosixUidGidTests) ... ok
test_setregid_neg1 (__main__.PosixUidGidTests) ... ok
test_setreuid (__main__.PosixUidGidTests) ... ok
test_setreuid_neg1 (__main__.PosixUidGidTests) ... ok
test_setuid (__main__.PosixUidGidTests) ... ok
test_listdir (__main__.Pep383Tests) ... ok
test_open (__main__.Pep383Tests) ... ok
test_stat (__main__.Pep383Tests) ... ok
test_CTRL_BREAK_EVENT (__main__.Win32KillTests) ... skipped 'Win32 specific tests'
test_CTRL_C_EVENT (__main__.Win32KillTests) ... skipped 'Win32 specific tests'
test_kill_int (__main__.Win32KillTests) ... skipped 'Win32 specific tests'
test_kill_sigterm (__main__.Win32KillTests) ... skipped 'Win32 specific tests'
test_directory_link (__main__.Win32SymlinkTests) ... skipped 'Win32 specific tests'
test_file_link (__main__.Win32SymlinkTests) ... skipped 'Win32 specific tests'
test_isdir_on_directory_link_to_missing_target (__main__.Win32SymlinkTests) ... skipped 'Win32 specific tests'
test_remove_directory_link_to_missing_target (__main__.Win32SymlinkTests) ... skipped 'Win32 specific tests'
test_rmdir_on_directory_link_to_missing_target (__main__.Win32SymlinkTests) ... skipped 'Win32 specific tests'
test_identity (__main__.FSEncodingTests) ... ok
test_nop (__main__.FSEncodingTests) ... ok
test_getppid (__main__.PidTests) ... ok
test_getlogin (__main__.LoginTests) ... skipped 'Skip due to platform/environment differences on *NIX buildbots'
test_link (__main__.LinkTests) ... ok
test_link_bytes (__main__.LinkTests) ... ok
test_unicode_name (__main__.LinkTests) ... ok
test_headers (__main__.TestSendfile) ... ok
test_invalid_offset (__main__.TestSendfile) ... ok
test_offset_overflow (__main__.TestSendfile) ... ok
test_send_at_certain_offset (__main__.TestSendfile) ... ok
test_send_whole_file (__main__.TestSendfile) ... ok
test_trailers (__main__.TestSendfile) ... ok
test_set_get_priority (__main__.ProgramPriorityTests) ... /Users/steffen/arena/code.extern.repos/py3k.hg/Lib/unittest/case.py:387: ResourceWarning: unclosed file <_io.BufferedReader name='@test_86650_tmp2'>
 function()
ok
----------------------------------------------------------------------
Ran 92 tests in 0.359s
OK (skipped=10)

> I'm at e79364a6bed8/[svn r88726].
> issue11351-5.patch applies and the test is ok, at least in respect 
> to sendfile:
Sorry, I'm still not used to the whole svn/mercurial change.
I applied your patch in r88729.
Let's see how the buildbot goes.

> Giampaolo Rodola' <g.rodola@gmail.com> added the comment:
> Let's see how the buildbot goes.
Italian Espressi are the best!
(Our local classical radio station plays Respighi's "Pini di Roma" 
almost every day ...)

=)

Buildbot ok, i hope it's ok for you all
that i close this heavy-load issue.
More fun further on up the road ...
from a non-MQ user.

Thanks.

Looks good to me and the test does now run successfully.
Note that sendfile() was introduced with OS X 10.5 so, like a number of other similar calls, os.sendfile() will not be present on 32-bit-only OS X installer builds which are built with a minimum deployment target of 10.3. It will be available on the 64-/32-bit installers which have a 10.6 minimum deployment target.