bpo-34521: Fix tests in test_socket to use correctly CMSG_LEN by pablogsal · Pull Request #9594 · python/cpython

pablogsal · 2018-09-26T21:07:35Z

After some failures in AMD64 FreeBSD CURRENT Debug 3.x buildbots
regarding tests in test_socket that are using
testFDPassSeparateMinSpace(), FreeBDS revision 337423 was pointed
out to be the reason the test started to fail.

A close examination of the manpage for cmsg_space(3) reveals that
the number of file descriptors needs to be taken into account when
using CMSG_LEN().

https://linux.die.net/man/3/cmsg_space

Example from the manpage. The code below passes an array of file descriptors over a UNIX domain socket using SCM_RIGHTS:

struct msghdr msg = {0};
struct cmsghdr *cmsg;
int myfds[NUM_FD]; /* Contains the file descriptors to pass. */
char buf[CMSG_SPACE(sizeof myfds)];  /* ancillary data buffer */
int *fdptr;

msg.msg_control = buf;
msg.msg_controllen = sizeof buf;
cmsg = CMSG_FIRSTHDR(&msg);
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS;
cmsg->cmsg_len = CMSG_LEN(sizeof(int) * NUM_FD);   <----------
/* Initialize the payload: */
fdptr = (int *) CMSG_DATA(cmsg);
memcpy(fdptr, myfds, NUM_FD * sizeof(int));
/* Sum of the length of all control messages in the buffer: */
msg.msg_controllen = cmsg->cmsg_len;

https://bugs.python.org/issue34521

After some failures in AMD64 FreeBSD CURRENT Debug 3.x buildbots regarding tests in test_socket that are using testFDPassSeparateMinSpace(), FreeBDS revision 337423 was pointed out to be the reason the test started to fail. A close examination of the manpage for cmsg_space(3) reveals that the number of file descriptors needs to be taken into account when using CMSG_LEN().

pablogsal · 2018-09-26T21:09:14Z

Manually checking on the buildbot itself reveals that all test for test_socket now pass:

CURRENT-amd64% uname -a
FreeBSD CURRENT-amd64 12.0-ALPHA6 FreeBSD 12.0-ALPHA6  r338678  amd64

CURRENT-amd64% ./python -m test test_socket
Run tests sequentially
0:00:00 load avg: 1.69 [1/1] test_socket

== Tests result: SUCCESS ==

1 test OK.

Total duration: 28 sec 611 ms
Tests result: SUCCESS

vstinner

LGTM.

pablogsal · 2018-09-27T09:25:54Z

I will wait for the buildbots before evaluating if we should backport this or not.

miss-islington · 2018-09-27T09:53:59Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington · 2018-09-27T09:53:59Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2018-09-27T09:53:59Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖

After some failures in AMD64 FreeBSD CURRENT Debug 3.x buildbots regarding tests in test_socket that are using testFDPassSeparateMinSpace(), FreeBDS revision 337423 was pointed out to be the reason the test started to fail. A close examination of the manpage for cmsg_space(3) reveals that the number of file descriptors needs to be taken into account when using CMSG_LEN(). This commit fixes tests in test_socket to use correctly CMSG_LEN, taking into account the number of FDs. (cherry picked from commit 7291108) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

bedevere-bot · 2018-09-27T09:54:12Z

GH-9597 is a backport of this pull request to the 3.7 branch.

After some failures in AMD64 FreeBSD CURRENT Debug 3.x buildbots regarding tests in test_socket that are using testFDPassSeparateMinSpace(), FreeBDS revision 337423 was pointed out to be the reason the test started to fail. A close examination of the manpage for cmsg_space(3) reveals that the number of file descriptors needs to be taken into account when using CMSG_LEN(). This commit fixes tests in test_socket to use correctly CMSG_LEN, taking into account the number of FDs. (cherry picked from commit 7291108) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

bedevere-bot · 2018-09-27T09:54:20Z

GH-9598 is a backport of this pull request to the 3.6 branch.

miss-islington · 2018-09-27T09:54:22Z

Sorry, @pablogsal, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 7291108d88ea31d205da4db19d202d6cbffc6d93 2.7

pablogsal · 2018-09-27T09:54:51Z

Good job, @miss-islington!

miss-islington · 2018-09-27T13:30:08Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

After some failures in AMD64 FreeBSD CURRENT Debug 3.x buildbots regarding tests in test_socket that are using testFDPassSeparateMinSpace(), FreeBDS revision 337423 was pointed out to be the reason the test started to fail. A close examination of the manpage for cmsg_space(3) reveals that the number of file descriptors needs to be taken into account when using CMSG_LEN(). This commit fixes tests in test_socket to use correctly CMSG_LEN, taking into account the number of FDs. (cherry picked from commit 7291108) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

vstinner · 2018-09-27T13:38:04Z

@pablogsal: It seems like the merged commit message lacks "bpo-34521".

pablogsal self-assigned this Sep 26, 2018

pablogsal requested a review from vstinner September 26, 2018 21:07

the-knights-who-say-ni added the CLA signed label Sep 26, 2018

bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Sep 26, 2018

pablogsal force-pushed the bpo34521 branch from 2f826fb to 80f5589 Compare September 26, 2018 21:08

pablogsal added the skip news label Sep 26, 2018

vstinner approved these changes Sep 27, 2018

View reviewed changes

pablogsal merged commit 7291108 into python:master Sep 27, 2018

pablogsal deleted the bpo34521 branch September 27, 2018 09:25

bedevere-bot removed the awaiting merge label Sep 27, 2018

pablogsal added needs backport to 3.6 labels Sep 27, 2018

bedevere-bot removed the needs backport to 3.7 label Sep 27, 2018

bedevere-bot removed the needs backport to 3.6 label Sep 27, 2018

pablogsal removed the needs backport to 2.7 label Sep 27, 2018

vstinner added the needs backport to 3.7 label Sep 27, 2018

vstinner removed the needs backport to 3.7 label Sep 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

bpo-34521: Fix tests in test_socket to use correctly CMSG_LEN#9594

bpo-34521: Fix tests in test_socket to use correctly CMSG_LEN#9594
pablogsal merged 1 commit into
python:masterfrom
pablogsal:bpo34521

pablogsal commented Sep 26, 2018 •

edited

Loading

Uh oh!

pablogsal commented Sep 26, 2018

Uh oh!

vstinner left a comment

Uh oh!

pablogsal commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

bedevere-bot commented Sep 27, 2018

Uh oh!

bedevere-bot commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

pablogsal commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

vstinner commented Sep 27, 2018 •

edited by bedevere-bot

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

pablogsal commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablogsal commented Sep 26, 2018

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

bedevere-bot commented Sep 27, 2018

Uh oh!

bedevere-bot commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

pablogsal commented Sep 27, 2018

Uh oh!

miss-islington commented Sep 27, 2018

Uh oh!

vstinner commented Sep 27, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pablogsal commented Sep 26, 2018 •

edited

Loading

vstinner commented Sep 27, 2018 •

edited by bedevere-bot

Loading