Add missing filedesc and fdescenttbl in FreeBSD

[GuillaumeGomez]

Types I need for sysinfo to allow to know how many open files a FreeBSD process has.

Item declarations can be found there:

• https://github.com/freebsd/freebsd-src/blob/58164dcb55d62ca73b5e550b8344bf61e2d8a47a/sys/sys/_lock.h
• https://github.com/freebsd/freebsd-src/blob/58164dcb55d62ca73b5e550b8344bf61e2d8a47a/sys/sys/filedesc.h
• https://github.com/freebsd/freebsd-src/blob/58164dcb55d62ca73b5e550b8344bf61e2d8a47a/sys/sys/_sx.h

[rustbot]

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tgross35]

Cc @asomers for FreeBSD

[asomers]

There are a lot of new definitions here. I suggest you check how many of these fields you truly need to use. For those that you don't, you can make the field private and opaque. That eases the maintenance burden on libc, and makes it possible to change the fields later if needed.

[asomers]

Are you defining it as "c_void" instead of struct file just so you won't have to add a definition for the latter, because you don't need it? In that case, I suggest you make the field private. That way it will be easier to change the field later.

[GuillaumeGomez]

Yes, gonna do that.

[asomers]

You should make this field private and opaque. It isn't used by userspace.

[GuillaumeGomez]

I made them private, but wdym by opaque? To get the size and alignment right, we still need to define the inner types, or do you mean something else?

[asomers]

You should be able to get the size and alignment correct with a suitably constructed array of bytes, longs, etc. But if that's too much work then it's ok to define the real structs instead.

[GuillaumeGomez]

I find it preferrable to have definitions matching C as it makes it easier for us to update it if changed but really as you prefer.

[tgross35]

For 1.0 we are going to make all opaque / padding fields MaybeUninit, that could be an option here

[GuillaumeGomez]

I didn't expect the segfault. XD

[tgross35]

FreeBSD13 CI has a spurious segfault. I have no idea why, haven't had a chance to look into it.

Retried the job

[GuillaumeGomez]

CI passed! \o/

[GuillaumeGomez]

@tgross35 Is there anything else to be done here?

[GuillaumeGomez]

Just realized two things:

• seqc_t type was unused so I removed it
• I forgot to update the kinfo_proc::ki_fd field type so I updated it as well

[GuillaumeGomez]

Rebased on main branch.

[tgross35]

I think this looks fine to me with one note that there should be a comment about why some of these are #[doc(hidden)]. It would be better to just have them non-pub or pub(crate) but I don't think the macros allow that, maybe it should be a FIXME(pub).

I'm not sure about the change to kinfo_proc; that is breaking so I'm not sure @asomers would want to backport it, in which case that needs to be split to a separate commit so I can backport the rest.

Could you add a permalink to the relevant headers in the PR descripton?

[GuillaumeGomez]

About the changes to kinfo_proc: you cannot access the data pointed by the pointer as is because it's a kernel pointer, you need to send this pointer to kvm_read to get the actual struct.

So although the pointer type changes, is it really an issue considering you cannot access the data as is?

[GuillaumeGomez]

Oh also I added the permalinks in the first comment.

[tgross35]

About the changes to kinfo_proc: you cannot access the data pointed by the pointer as is because it's a kernel pointer, you need to send this pointer to kvm_read to get the actual struct.

So although the pointer type changes, is it really an issue considering you cannot access the data as is?

Good point, it's probably not a problem in practice then. I'm fine with backporting everything as long as Alan okays it.

[GuillaumeGomez]

Thanks, that'd allow me to remove a lot of code from sysinfo. :)

[asomers]

I'm ok with backporting the kinfo_proc change to the 0.2 branch.

[tgross35]

@asomers was there anything else you wanted here or is this good to merge?

[asomers]

@asomers was there anything else you wanted here or is this good to merge?

I would prefer that types like sx weren't declared, since they will never be used in userland. It would be better to make those fields just an opaque byte array. But if you like it this way instead, I'm ok with it.

[GuillaumeGomez]

I'd prefer to keep the struct declaration. In future version, we can make the macro allow to have private types.

[tgross35]

I think that is fine, should be easier than calculating by hand. We've had some CI problems this week but they should be fixed now so this is all set.

Would you be able to squash?

[GuillaumeGomez]

Sure, done.