Re: [PATCH] dbus: fix appending on dict for types other than string
On Tue, Jan 4, 2011 at 3:57 PM, Marcel Holtmann <marcel(a)holtmann.org> wrote:
Hi Denis,
> >>
> >> dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING,
> >> &(val_array[i + 0]));
> >> - dbus_message_iter_append_basic(&entry, type,
> >> - &(val_array[i + 1]));
> >> + if (type == DBUS_TYPE_STRING) {
> >> + dbus_message_iter_append_basic(&entry, type,
> >> + &(val_array[i +
1]));
> >> + } else {
> >> + dbus_message_iter_append_basic(&entry, type,
> >> + val_array[i + 1]);
> >> + }
> >
> > this does looks a bit hackish. Can you put it into context please. It
> > might be that our call chain is wrong.
> >
>
> We only ever needed (and thus ever added) support for string:string
> dicts. E.g. ones that could be g_strfreev-ed. No other dicts were
> supported by this function.
so it is the g_strfreev part that makes this hackish. Fine by me then,
but we might need to comment on why string arrays in this case are
special.
I don't think so. You use g_strfreev if you are using { string, string
} dict to free all strings. Why is this hackish?
Even if we had another interface that didn't required the g_strfreev,
the issue with calling dbus_message_iter_append_basic() would not
change once we continue using a void** to store pointers to different
types at the same time..
Lucas De Marchi