Bug 3931

Created attachment 3199 [details] [review]
Don't share displays across tet_fork, and make event test more deterministic

Patch was eaten by the hard disk crash.  Can you reattach.  Thanks.

Created attachment 2913 [details] [review]
Fix 64-bit alignment problems

is there a better way to do the defined(__FreeBSD__) using a functionality test
insted? Does __FreeBSD__ define LOCAL_CREDS but the code path doesn't work? 
Otherwise the patch looks good.

Unfortunately, FreeBSD does now define LOCAL_CREDS, but it does not provide the
pid, so it cannot be used.  I suppose you could change the test to always use
cmsgcred if HAVE_CMSGCRED is defined, then fallback to LOCAL_CREDS if it is not
defined, and if LOCAL_CREDS is defined.  That would work on FreeBSD.

Is not providing the pid a bug in FreeBSD?  In that case the #ifdef makes sense
probably.

It's not a bug in FreeBSD.  NetBSD has the same behavior.  In fact, I have not
found an implementation of LOCAL_CREDS that does allow one to obtain the pid
(struct sockcred does not contain a pid member in NetBSD, FreeBSD, QNX, or
OpenBSD).  I think LOCAL_CREDS might not be sufficient for D-BUS.

>I suppose you could change the test to always use
>cmsgcred if HAVE_CMSGCRED is defined, then fallback to 
>LOCAL_CREDS if it is not
>defined, and if LOCAL_CREDS is defined.

Can you do that and post a new patch? Thanks.

Bugzilla Upgrade Mass Bug Change

NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO.

  - benjsc
    fd.o Wrangler

Why add __FreeBSD__ ? Isn't your patch also correct without?  (It does to me...)

Sorry - I didn't see the rest...


I just came to this bug because current git head uses

sizeof(struct cmsgcred)

ifdef LOCAL_CREDS, and LOCAL_CREDS doesn't have struct cmsgcred.

Something like

#ifdef HAVE_CMSGCRED
#define CRED_SIZE (CMSG_SPACE (sizeof (struct cmsgcred)))
  union {
    struct cmsghdr hdr;
    char cred[CRED_SIZE];
  } cmsg;

#elif defined(LOCAL_CREDS)
/* Assuming one group */
#define CRED_SIZE (sizeof (struct sockcred))
  struct {
    struct cmsghdr hdr;  
    struct sockcred cred; 
  } cmsg;
#endif


and using CRED_SIZE ifdef HAVE_CMSGCRED || LOCAL_CREDS should do the trick,
but then CMSG_SPACE or CMSG_LEN ?

This bug can be closed.  The code in dbus git is working fine.

OK - I'll open a new bug about the inability to compile if LOCAL_CREDS and !HAVE_CMSGCRED as then you hit sizeof(struct cmsgcred) which obviously is only defined if you HAVE_CMSGCRED... (this is git-head 46df8b52d970...)

dbus-sysdeps-unix.c:1485 (and 1523) error: invalid application of 'sizeof' to incomplete type 'struct cmsgcred'