Hi Inaky,

On 07/29/2010 04:47 PM, Inaky Perez-Gonzalez wrote:
> On Tue, 2010-07-27 at 10:08 -0700, Denis Kenzior wrote:
>> Hi Inaky,
>> 
>>> +/* + * Encapsulate information needed to export an SMS message 
>>> over D-Bus. + * + * @dbus_path: path of the object in the D-Bus
>>> + * @dbus_msg: message this originated at + */ +struct 
>>> sms_msg_dbus_data { +	char *dbus_path; +	DBusMessage *dbus_msg; 
>>> +}; +
>> 
>> I don't really see a point for this structure.  When you send an 
>> SMS, assuming the arguments and format were valid, you return 
>> straight away. Thus SendMessage should be marked as not being
>> ASYNC and storing of DBusMessage is unnecessary.  Rest can be
>> handled by txq_submit and the dbus path can be easily generated
>> based on the UUID.
> 
> First it serves as transitional storage -- later in the commit 
> sequences the dbus_msg is removed as it is, as you say, unneeded
> once all the code is made sync. If at the end only one member is
> left (dbus_msg)

Don't get me wrong, if a structure is necessary in the end, so be it.
However, during the review I only see a structure being introduced that
is totally unnecessary.  You can simply pass dbus_path around as
userdata.

If/when this structure does become necessary, feel free to
introduce it.

> 
> In our discussions to plan this stuff you said you wanted the D-Bus 
> unique name to contain the UUID and the number of PDUs 
> (/modemX/UUID_PDUs). That means I need to access the SMS object if I
>  want to generate the name on the fly. I need a backpointer for
> that.
> 
> Even if we remove the _PDUs requirement:

There is no such requirement.  If I gave you this idea, then I must have
not been clear enough.  I meant that PDUs would be used to generate the
UUID.

Regards,
-Denis