Skip to content

Conversation

nykula
Copy link
Contributor

@nykula nykula commented Jan 7, 2019

Fixes #192 similarly to what audioipc-2 does. Now builds on musl.

@highfive
Copy link
Collaborator

highfive commented Jan 7, 2019

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @emilio (or someone else) soon.

@highfive
Copy link
Collaborator

highfive commented Jan 7, 2019

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

msghdr.msg_iovlen = iovec.len() as IovLen;
msghdr.msg_control = cmsg_buffer as *mut c_void;
msghdr.msg_controllen = cmsg_space as MsgControlLen;
msghdr.msg_flags = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you wrap this into a function? E.g. new_msghdr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, re-uploaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the noise, I seem to misunderstand how to extract this piece. Will get back to this.

msg_controllen: cmsg_length as MsgControlLen,
msg_flags: 0,
},
msghdr: new_msghdr(&mut iovec, cmsg_buffer, cmsg_length as MsgControlLen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iovec is already an &mut [iovec] no need for the &mut.

Copy link
Contributor

@dlrobertson dlrobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jdm
Copy link
Member

jdm commented Jan 9, 2019

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit cee64a7 has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit cee64a7 with merge 621f787...

bors-servo pushed a commit that referenced this pull request Jan 9, 2019
unix: Check libc not os, and zero msghdr private

Fixes #192 similarly to what [audioipc-2](https://github.com/djg/audioipc-2/blob/1033cf7e4b5d14a138004bbb8708b2a1c3821939/audioipc/src/msg.rs#L79) does. Now builds on musl.
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis, status-appveyor
Approved by: jdm
Pushing 621f787 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants