Re: patch review comments

On Tue, 2004-05-11 at 18:48 -0400, Havoc Pennington wrote:
> 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.

        Fixed.

> 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
> called.

        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,

        Thanks,

                Michael [ who considers his next cut ]

-- 
 michael-YszlSufgerXQT0dZR+AlfA@xxxxxxxxxxxxxxxx  <><, Pseudo Engineer, 
itinerant idiot