On Thu, 2012-02-16 at 14:29 +0100, Patrick Ohly wrote:
> On Mon, 2012-02-13 at 14:50 +0100, Chris Kühl wrote:
> > The above mentioned changes compile and are being tested now. I'll get
> > back to you with the results of that.
>
> How is that going?
>
> I'm now looking more seriously at the code myself. One thing which
> caught my attention is how blocking D-Bus calls are used in
> syncevo-dbus-server.
>
> For example, ConnectionResource::process(): it passes the incoming data
> on to the helper process with a blocking call. Isn't that a big no-no?
... and error handling seems to be missing entirely. If the helper
fails, the class just ignores the error string.
This is going to be a bit better with the new blocking call operator,
because then errors encountered during the execution of the D-Bus call
or returned by the peer will be thrown as runtime exceptions in the
context of the ConnectionResource::process(), leading to a failure of
that call itself.
Speaking of errors returned by the peer:
Return_t sendAndReturn(DBusMessagePtr &msg)
{
DBusErrorCXX error;
// Constructor steals reference, reset() doesn't!
// Therefore use constructor+copy instead of reset().
DBusMessagePtr reply =
DBusMessagePtr(dbus_connection_send_with_reply_and_block(m_conn.get(),
msg.get(), -1, &error));
if (!reply) {
error.throwFailure(m_method);
}
return CallTraits::demarshal(reply, m_conn);
}
static Return_t demarshal(DBusMessagePtr &reply, const DBusConnectionPtr
&conn)
{
typename dbus_traits<R1>::host_type r;
ExtractArgs(conn.get(), reply.get()) >> Get<R1>(r);
return r;
}
"reply" may be an error message. That must be checked instead of trying
to extract return values that won't be there.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
