SCTP support for FreeBSD, NetBSD

[jack-pappas]

This PR adds constants, structs, and functions for SCTP networking support on FreeBSD and NetBSD.

[jack-pappas]

@alexcrichton I'm fairly new to Rust, so I have a couple of questions for you about this PR:

• A number of the structures defined here are using __packed in the C headers (from the OS). libc's s macro applies #[repr(C)] to structs defined within it; given these structs were using __packed in their C representation, I think that means the rust versions need to use #[repr(C, packed)]? If so, how should I add that to the struct definitions? Would it make sense to define another macro ps to use for packed structs, or is there a better approach?
• The CI builds are currently failing because some of the new types I defined are unions and use the relatively new union support in Rust, while the CI runs appear to be using an older version of rustc. Are unions allowed in libc, or is this crate targeting an older version of the language standard for compatibility reasons? This seems a little similar to Problem in implementing siginfo_t for Linux #716, except the signature in question there already exists with a non-union representation whereas this PR contains only additions (so there aren't any backwards-compatibility concerns with existing type definitions).

[krytarowski]

without sys/ here and in the next line

[jack-pappas]

You're right, I'll fix the comment -- thanks!

[krytarowski]

Thanks for the NetBSD patch!

[alexcrichton]

Thanks for the PR @jack-pappas!

To help answer your questions, I think you should be able to add #[repr(packed)] on the structs I think? I believe the macro has support for collecting those attributes and placing them on the structs.

For union I think it's a little too soon for libc to start using that type, but would it be possible to have an opaque type for now that we can extend to be a union later?

[jack-pappas]

Thank you for your response Alex. I'll try adding #[repr(packed)] to the structs.

As for union -- we could use an opaque type instead for now, I'll give that a try. Would it be possible to include both representations through a conditional compilation attribute? E.g. in build.rs if there's a way to detect the rustc version being used, we could set a have_unions compilation symbol, then choose either the union-based representations or the opaque type representation using #[cfg(have_unions)].

[alexcrichton]

Nah right now we avoid that sort of conditional compilation, it's easier and more robust to stick to one source.

[jack-pappas]

I've added #[repr(packed)] to the structs that needed it. I still need to change the unions to be opaque types instead -- will try to do that this weekend.

[jack-pappas]

I changed the main type using union (the SCTP notification type) to be a struct instead, in such a way it should be fairly straightforward for consumers to change their code later on once the type can be changed back to a union. I commented out a couple of other uses of union which are logging-related and are optional anyway -- their absence won't stop clients from implementing SCTP networking code.

There are some errors remaining, which appear to be due to some of the new structs using the equivalent of a C99 flexible array; they can't implement the Copy trait, and that's causing the build to fail. How should I go about fixing that?

[alexcrichton]

Oh this I don't think is FFI-safe or possible for us to define, for something like this historically I've just defined a 0-length array here instead of a DST array

[alexcrichton]

Does fixing this fix the all the remaining errors?

[jack-pappas]

I can check to see if using a zero-length array instead fixes the errors, but why would using the flexible / dynamically-sized arrays be FFI-unsafe? If the definition of the struct in the Rust code matches what's in the C header, isn't that all that's needed for FFI-compatibility?

It seems much more unsafe to use a zero-length array -- consumers of the type will need to use that array like a raw pointer. If the struct is moved or copied somewhere else in memory, the array data won't be copied along with it, and consumers of the struct won't have any real way to tell whether they're looking at the "original" struct with valid data in the array or a copy that's been moved and now has invalid/garbage data where it's expecting the array to be.

[alexcrichton]

Sorry I don't have a lot of time to go in depth, but the errors here basically show how a dynamically sized array is a no-go. If it worked that'd be great and for sure better than a zero-length array, but I don't believe it works.

[bors]

☔ The latest upstream changes (presumably #809) made this pull request unmergeable. Please resolve the merge conflicts.

[jack-pappas]

I've fixed the merge conflicts and made the suggested changes for the arrays.

I'm still getting two linter errors:

src/unix/bsd/freebsdlike/freebsd/mod.rs:37 - struct found after constant when it belongs before
src/unix/bsd/netbsdlike/netbsd/mod.rs:19 - struct found after constant when it belongs before

Does that mean it's not possible to define constants used for the array lengths within the struct definitions? If not, I'll just change the arrays in the structs to inline the constant values, but figured it'd be nicer to preserve the constant as in the original C headers.

[alexcrichton]

Oh that's just a stylistic lint which just requires a few things to move around.

[bors]

☔ The latest upstream changes (presumably #802) made this pull request unmergeable. Please resolve the merge conflicts.

[jack-pappas]

@alexcrichton One of the structs (as defined in the C headers) has a field called type. How should that be handled for libc? I can’t name it something else because the tests complain about not being able to find the field, and it doesn’t appear Rust has anything like C#’s @ prefix to allow reserved keywords to be used as identifiers.

There are also a few remaining issues in the tests where the generated C code is doing sizeof(...) on zero-length array fields and the C compiler is emitting an error for those. Is there something else I should be doing for these fields to make them work?

[alexcrichton]

Ah I've typically just been callng such field type_ (underscore at the end). You can rename the field in the tests (see libc-test/build.rs)

I think the same script should enable you to ignore certain fields in particular structs which may help?

[bors]

☔ The latest upstream changes (presumably #840) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #867) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

I'm gonna close this due to inactivity, but it can always be resubmitted!