Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(19)

Issue 3825042: Reply an error message to sender, if ibus-daemon can not forward method call message successfully. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 2 months ago by Peng
Modified:
15 years, 2 months ago
Reviewers:
Yusuke Sato
CC:
satorux
Base URL:
git@github.com:/ibus/ibus.git@master
Visibility:
Public.

Description

Reply an error message to sender, if ibus-daemon can not forward method call message successfully. BUG=none TEST=manually

Patch Set 1 #

Total comments: 4

Patch Set 2 : Update CL #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -43 lines) Patch
M bus/dbusimpl.c View 1 6 chunks +64 lines, -43 lines 0 comments Download

Messages

Total messages: 4
Peng
15 years, 2 months ago (2010-12-27 20:08:52 UTC) #1
Yusuke Sato
LGTM, but I have one question (see below.) http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c File bus/dbusimpl.c (right): http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c#newcode887 bus/dbusimpl.c:887: g_object_unref ...
15 years, 2 months ago (2010-12-28 01:06:54 UTC) #2
Peng
Update CL http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c File bus/dbusimpl.c (right): http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c#newcode887 bus/dbusimpl.c:887: g_object_unref (message); On 2010/12/28 01:06:54, Yusuke Sato ...
15 years, 2 months ago (2010-12-28 19:28:29 UTC) #3
Yusuke Sato
15 years, 2 months ago (2010-12-28 23:21:57 UTC) #4
LGTM, thanks.

On 2010/12/28 19:28:29, Shawn.P.Huang wrote:
> Update CL
> 
> http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c
> File bus/dbusimpl.c (right):
> 
> http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c#newcode887
> bus/dbusimpl.c:887: g_object_unref (message);
> On 2010/12/28 01:06:54, Yusuke Sato wrote:
> > How about changing line 887-888 to something like:
> > 
> > g_warning ("Unknown method call: destination=NULL, interface=...,
member=...,
> > path=...", ...);
> > return message;
> > 
> > so that the GDbus library could automatically return an error reply to
unknown
> > method calls like GetAll in DBus.Properties?
> I think so. I will use this way to handle dest==NULL.
> > 
> > If I remember correctly, the GetAll method call
> > (http://codereview.appspot.com/3836042/) is sent with NULL destination and
> > therefore is not handled by bus_dbus_impl_forward_message_idle_cb which you
> > modified in this CL.
> 
> http://codereview.appspot.com/3825042/diff/1/bus/dbusimpl.c#newcode1062
> bus/dbusimpl.c:1062: "No service name is '%s'.", destination);
> On 2010/12/28 01:06:54, Yusuke Sato wrote:
> > nitpick: No -> The ??
> 
> Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b