Add FreeBSD support, round 1

[fluffykhv]

Port patches from FreeBSD ports tree (https://github.com/freebsd/freebsd-ports/tree/main/security/sssd2/files), round one

[alexey-tikhonov]

Hi @fluffykhv,
thank you for the contribution.

Is "Add FreeBSD support, round 1" a single huge patch in FreeBSD repo?
If this is a set of patches, wouldn't it make sense to have a commit per patch to make the review easier?

[emaste]

Patches in the FreeBSD ports collection are typically per-file rather than per-logical-change, so it may not be straightforward to recreate an incremental patch set. If you look at the URL above you'll see the per-file patches. This is admittedly not ideal from the perspective of collaboration with an upstream.

Looking at the history (e.g. https://github.com/freebsd/freebsd-ports/commits/main/security/sssd2/files) gives some insight, and you can see some logical changes that could be applied individually e.g. freebsd/freebsd-ports@9a40ff8.

If you want a patch set of logical changes we'll have to curate the patches in the ports tree.

[alexey-tikhonov]

Well, that's going to be tough...

As I'm not familiar with FreeBSD, it would help greatly if you could provide a rationale for additional includes.
What especially strikes out:

• <sys/param.h> (what's there?)
• <sys/types.h> (why?)
• <sys/time.h> (why?)
  everywhere.

I think I'll cherry pick trivial / non-questionable patches in a separate PR, and once it's merged, you'll have to rebase.
But in general it would be great to have as little #ifdef __FreeBSD__ in the code as feasible.

In case of headers that might be finding a common ground where possible and, where it's impossible, putting conditional includes in a single common header to reduce clutter overall.

Wrt prctl(): there is HAVE_PRCTL - can it be used instead?
Btw, patches do not touch https://github.com/SSSD/sssd/blob/master/src/util/capabilities.c#L23 -- is this because FreeBSD didn't pick sssd-2.10+ yet?

[arrowd]

As I'm not familiar with FreeBSD, it would help greatly if you could provide a rationale for additional includes.

The general answer is - the transitive dependencies between FreeBSD headers are different from the Linux ones. It often comes up when porting software for FreeBSD that a single #include <foo.h> line works for Linux, because at also pulls in bar.h, but on FreeBSD these headers are decoupled from each other, so you have to include both in the source file. I think Dima can provide you exact error messaged if you're interested.

In case of headers that might be finding a common ground where possible and, where it's impossible, putting conditional includes in a single common header to reduce clutter overall.

If I'm not mistaken, POSIX dictates what header should be included to be able to use a given struct or function. So it is not matter of finding a common ground between different OSes, but simply following the standard.

Btw, patches do not touch https://github.com/SSSD/sssd/blob/master/src/util/capabilities.c#L23 -- is this because FreeBSD didn't pick sssd-2.10+ yet?

Yes, we're still at 2.9 at the moment. Hopefully we'll catch up soon.

[alexey-tikhonov]

If I'm not mistaken, POSIX dictates what header should be included to be able to use a given struct or function.

Where it is POSIX mandated it will be "non questionable" and it is where some help with pointers would be appreciated. It will be just WAY slower for non FreeBSD user to figure out a reason for inclusion of particular header.