search

godbus / dbus

Native Go bindings for D-Bus
BSD 2-Clause "Simplified" License
976 stars 225 forks source link

freebsd: Remove the cgo dependency from transport_unixcred_freebsd.go #332

Closed dfr closed 1 year ago

dfr commented 2 years ago

This also stops the code from wrongly using sizeof(struct ucred) which is unrelated to SCM_CREDS. The size of the correct cmsgcreds structure is hard-coded here - this ABI has not changed for ~25 years.

The added dependency on golang.org/x/sys/unix could be avoided if necessary, e.g. by using unsafe.Sizeof(uintptr).

Fixes #331

dfr commented 1 year ago

Rebased

dfr commented 1 year ago

@guelfey PTAL

guelfey commented 1 year ago

Sorry for the delay. Also see https://github.com/godbus/dbus/pull/318. As mentioned there, IMO the details of this struct should be handled in sys/unix; I opened https://github.com/golang/go/issues/51711 for that, but didn't get around to fixing that. This would make it a bit more proper / "future-proof" as there is tooling there to create these struct / size definitions for all architectures (even though here it wouldn't matter).

dfr commented 1 year ago

No worries about the delay - thanks for taking a look. I agree that it would be best to have the struct definition in x/sys but as I mentioned above, this ABI is extremely stable and is the same across 32bit and 64bit platforms so perhaps that doesn't have to block?

Reading the golang issue, I have a very slight preference to call the struct Cmsgcreds - while the APIs are similar, they are not the same as you point out and Cmsgcreds additionally gives access to all the user's groups. I can comment there if that helps and I'm happy to make a PR along the lines suggested using whichever name consensus prefers.

guelfey commented 1 year ago

Hm fair point, unless some future architecture will switch to 64 bits for process IDs or similar :smile: but that really rather seems unlikely to me