patch review comments
michael at ximian.com
Fri May 28 05:41:22 PDT 2004
On Tue, 2004-05-11 at 18:48 -0400, Havoc Pennington wrote:
> Factoring out a bunch of cut-and-paste code, thanks, seems very
> 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
Sure, this was to avoid an over-large patch, since binning the
bogus prototypes requires some code re-ordering action; did the
re-ordering though & removed the warning.
> 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.
Tested - works fine.
> 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
Moved it down the file into those guards, I've left it
declared in the header instead of renaming to an
'_internal_do_not_use_' variant - IMHO this is a useful function to
export generally - at least as useful as the other
dbus_string_parse_uint type variants I think.
Committed it anyhow,
Michael [ who considers his next cut ]
michael at ximian.com <><, Pseudo Engineer, itinerant idiot
More information about the dbus