pam-sys
pam-sys copied to clipboard
1wilkens
Add Support for FreeBSD
Hey thanks for the PR! In general I am in favor of supporting as many operating systems as possible, so your work is appreciated. However I am not sure about some changes in your PR:
- Why should we remove usage line related to
libc? Is that not needed anymore? - Why should we use explicit white-listing of the
PAM_Xsymbols instead of the wildcard? - Why should we manually define the enum values you put in
lib.rs? Shouldn't they be generated appropriately for each platform (given the header is correct)?
Hey thanks for the PR!
Aboslutely! Thanks for creating the Rust bindings for libpam :)
Why should we remove usage line related to libc? Is that not needed anymore?
I don't think so, and it causes an error on FreeBSD (some included libc types are undefined there).
Why should we use explicit white-listing of the PAM_X symbols instead of the wildcard
There is a constant defined in FreeBSD (PAM_SILENT) that gets interpreted as a signed integer (libc::c_int/i32), causing the other constants in its grouping to also be defined as signed integers (they should be libc::c_uint/u32). To prevent that, I had to remove the wildcard, whitelist the constants that don't cause issues, and manually define the remaining PAM_X constants.
It's ugly, I know... I tried a few other things, and the manual definition was the only thing that worked.
Why should we manually define the enum values you put in lib.rs? Shouldn't they be generated appropriately for each platform (given the header is correct)?
See my answer above to your second question.
Sorry I am a little busy right now but have not forgotten about this! Will come back to that soonish!
Sorry for the late response @orvij. I pushed some changes that should fix the build for FreeBSD while retaining the wildcard usage.
I removed the problematic import from libc and manually defined PAM_SILENT via .raw_line(..).
However, after some additional digging I am not sure if the constants should be u32 as all functions on Linux and also openPAM/FreeBSD just take int and not uint in their signature. In fact that was the reason for the commit, that configured bindgen to default to signed integers for enums.
As I don't have access to a FreeBSD box at the moment, could you try out current master to check if it builds correctly?
Some more progress! I added CI for FreeBSD via cirrus-ci and refactored the build script some more to split up platform-specific code some more. Additionally I added security/openpam.h to wrapper.h when compiling under FreeBSD. The build passes after some fiddling but I cannot verify if the constants are in the intended format. @orvij could you build master again to see if your issue is solved? I'd really want to avoid manual definitions in lib.rs, however we can make many additional adjustments in the platform-specific part of build.rs if required :)