Created attachment 1137 [details] [review]
Add FreeBSD support to dbus

Where do HAVE_CMSGCRED and LOCAL_CREDS get defined?

For 0xff I don't think changing the test suite is right; either we remove the
code to handle 0xff and return an error for that on all platforms, or we should
add code to handle it on freebsd. I can't say offhand if dbus relies on being
able to parse hex numbers.

HAVE_CMSGCRED is defined in dbus' configure.  LOCAL_CREDS is defined by the
supporting operating systems.  This patch was adapted from the same patch I
submitted for gnome-keyring (however, it has not been committed there).

As for the test suite, parsing hex numbers is not supported on FreeBSD 4.X, but
it is supported on 5.X and higher.  I was told by David Zeuthen that parsing hex
numbers wasn't really critical to dbus.  However, those hunks should have been
omitted.  Sorry.

Havoc: The existing code in D-BUS already uses HAVE_CMSGCRED and LOCAL_CREDS in
_dbus_read_credentials_unix_socket.  This is just the sending parallel to the
existing code as far as I can tell.  So it seems sane to me (minus the test
suite hex bits).

Created attachment 1561 [details] [review]
Supercedes previous patch, and fixes alignment problems on non-i386 platforms

I think the current code was supposed to handle OS X - is this patch likely to
break OS X if that was working?

No.  The new code should work just as well on OS X, as well as other BSD-based OSes.

Comment on attachment 1561 [details] [review]
Supercedes previous patch, and fixes alignment problems on non-i386 platforms

It might be worthwhile to use _DBUS_ZERO rather than memset, though this
comment also applies to the existing code. The purpose of this is to avoid the
common reverse-args-of-memset bug.

The patch looks fine to me though I really don't understand it ;-) if it works
for you on BSD then OK.

I'm assuming the dbus_bindings.pyx.in change is unrelated, maybe be sure you
don't check that in accidentally.

The patch works for me (as best I can tell).  We use the same code in
gnome-keyring, and that definitely works (it didn't work without the modifications).

As for the Python patch, that is needed also.  Without it, the Python bindings
do not build on FreeBSD.

OK, the python change is fine with me though I don't know anything about nor
normally maintain the python bindings. You could put it in and see if someone
complains.

Thanks

I added this patch (fixing one conflict in the process); Joe, it'd be nice if
you could double-check it works as expected.

Actually, the FreeBSD support patch has changed a bit since this bug was
initially submitted.  As soon as 0.35 comes out, I'll roll up a new patch, and
submit it.  What you committed was correct, but since then, FreeBSD received
LOCAL_CREDS support.  Unfortunately, this is not sufficient for D-BUS, so an
additional check on __FreeBSD__ must be added.  On top of that, I've added
kqueue support to mimic the dnotify functionality in 0.34.

Thanks for looking at this.