[PATCH] dbus: clear guid_from_server if send_negotiate_unix_fd failed

[Simon McVittie]

On 27/02/14 01:37, rongqing.li at windriver.com wrote:
> bus-test dispatch test failed with below information:

Please open a bug on bugs.freedesktop.org, product "dbus", where this
can be discussed in more detail. This isn't the kernel; we prefer to use
bug-tracking, rather than making the mailing list high-traffic by going
into detail about individual patches.

We don't need the entire traceback in the commit message, as long as
there's a brief summary of what happens, and a bug reference for more
detail/discussion.

No need to reply to this email directly, but please say on the bug what
your reply would have been :-)

>     6363: assertion failed "_dbus_string_get_length (& DBUS_AUTH_CLIENT (auth)->guid_from_server) == 0" file "dbus-auth.c" line 1545 function process_ok

What platform is this on? If this code is wrong (which is entirely
possible), it isn't clear to me why this failure mode hasn't been seen
before.

Which version (which git commit) is this?

> Once send_negotiate_unix_fd failed, this failure will happen, since
> auth->guid_from_server has been set to some value before
> send_negotiate_unix_fd. send_negotiate_unix_fd failure will lead to
> this auth be handled by process_ok again, but this
> auth->guid_from_server is not zero.

So is the problem essentially that process_ok() should be idempotent,
because we might retry it after running out of memory on a previous attempt?

If so, an out-of-memory condition in send_begin() probably needs the
same treatment. I'd also like a comment "reset to the state we were in
on entry to this function, so we can try again when we have more memory"
or something.

An alternative approach would be to remove the assertion on entry, and
just truncate guid_from_server to zero length instead, with a comment
that it might be left over from a previous attempt that ran out of memory.

process_ok() appears to be making life complicated for itself by using
guid_from_server as scratch space for hex decoding as well as permanent
storage for the GUID. Since we don't actually seem to need the decoded
hex, on the master branch (but not dbus-1.x stable branches) I'd be
tempted to add a _dbus_string_validate_hex() and use that instead.

> +  if (auth->unix_fd_possible) {

Coding style: we aim for GNU/GNOME style, please do the same (e.g.
_dbus_auth_encode_data() in the same file is an example of correct
indentation).

    S