patch review comments

[Havoc Pennington]

Hi,

Various patches here, thanks everyone. I'm pretty sure I didn't get all
the outstanding stuff, maybe resend or file in bugzilla.

Havoc

Kristian patch to the auth stuff
===

This looks pretty reasonable, I think factoring out the send_* functions
is a nice code cleanup as are the other changes. 

The split_string() function should have a linefeed before the function
name, and is generally yucky string stuff but OK in test code. Should be
sure it only builds with tests enabled.

I didn't see anything else wrong here.

John patch on OBJECT_PATH
===

I think probably we should return the object path as a flat string, not
the string array. Also, the TODO item covers OBJECT_PATH_ARRAY but the
patch only covers OBJECT_PATH

Michael marshaling cleanup patch
===

Factoring out a bunch of cut-and-paste code, thanks, seems very
sensible.

In demarshal_basic_type I would use get_data accessor rather than
DBusRealString. If it's a performance issue let's just inline/macroize
the accessor.

There's a #warning left over in dbus-message.c

Should test the build without INT64 to be sure it still works right.
Note that INT64-containing messages still have to validate, however apps
don't have to be able to get at the values.

dbus_string_parse_basic_type should be in the
only-builds-with-tests-enabled code, possibly not in dbus-string.c, but
in any case inside the DBUS_BUILD_TESTS or whatever the macro is called.

Olivier patch for OwnerChanged
===

On the issue of dbus_message_get_args() not supporting STRING_OR_NIL,
indeed I think we need to deal with this. The problem with silently
assuming any string is STRING_OR_NIL is that apps would have to manually
check != NULL on all strings. My best suggestion is to introduce OR_NIL
into type codes? Conceivably we should sort this out before applying the
patch since it has you jumping through hoops in the dispatch.c test
code.

In at least two places you're passing a possibly-null string to "%s",
which only works with GNU printf. Have to do s ? s : "null"

I'd rather not do the anonymous inlined enums, kind of unclear. Just
split out the enums from the struct declarations.

SMTHG should be spelled out.

There's a comment /* WEIRD */ that makes me nervous about the code there
but I don't really know what was considered weird ;-)