Prevent MsgHdr::with_control with empty buffer causes error in macOS

[youknowone]

Otherwise panic when cfg(debug_assertions) will be another good prevention.

[Thomasdezeeuw]

Are there valid reasons to send an empty control value? Or is this always programmer error?

[youknowone]

To tell about the valid reasons, I am not familiar with low level socket API that much.
At least it was a programmer error for my case. Is panic better?

[Thomasdezeeuw]

To tell about the valid reasons, I am not familiar with low level socket API that much. At least it was a programmer error for my case. Is panic better?

No, I would say not doing anything would be better. We're not going to introduce panics in our API, but hiding errors/problematic code is also not something we want.

[youknowone]

It was a small glitch but painful to debug.
I finally could find the buffer is wrong, but it was hidden inside socket2 crate.
As a library user, I could see something goes wrong on sendmsg but not on with_control. If with_control is not expected to be called with empty buffer by nature, it could be warned at the moment, instead of deferring it.
Maybe the other side of the reason is Debug for MsgHdr was useless to look in it and didn't display its contents. The combination of blackboxed MsgHdr and deferred error took me long time to look around control_buffer.

I changed the patch to debug_assert!() make it neither panic(in real use) nor hiding and deferring error. Please take a review if you agree it is a UX caveat.

[Thomasdezeeuw]

It was a small glitch but painful to debug. I finally could find the buffer is wrong, but it was hidden inside socket2 crate.

Socket2 doesn't hide anything, you pass it an invalid buffer, it will pass that buffer along to the OS.

As a library user, I could see something goes wrong on sendmsg but not on with_control. If with_control is not expected to be called with empty buffer by nature, it could be warned at the moment, instead of deferring it. Maybe the other side of the reason is Debug for MsgHdr was useless to look in it and didn't display its contents. The combination of blackboxed MsgHdr and deferred error took me long time to look around control_buffer.

The layout of MsgHdr depends on the underlying OS, it's not a black box.

I changed the patch to debug_assert!() make it neither panic(in real use) nor hiding and deferring error. Please take a review if you agree it is a UX caveat.

Socket2 is just wrapper around the OS interfaces, so I don't think this fits in socket2.

[youknowone]

I understand your concern. Is there any plan to implement better Debug for MagHdr in that case? Debug can be platform-specific implementation and it will not affect any actual behavior. Minor platforms still can use current implementation

[Thomasdezeeuw]

I understand your concern. Is there any plan to implement better Debug for MagHdr in that case? Debug can be platform-specific implementation and it will not affect any actual behavior. Minor platforms still can use current implementation

A pr for that would be accepted.

[Thomasdezeeuw]

Closing this since I still don't think it's something we want.