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

Issue 6589065: ibus-daemon: use GFileMonitor instead of polling (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by Daiki Ueno
Modified:
1 year, 5 months ago
Reviewers:
shawn.p.huang
Base URL:
git@github.com:ibus/ibus.git@master
Visibility:
Public.

Description

ibus-daemon: use GFileMonitor instead of polling


BUG=Issue#1514

Committed: 1e6c42f

Patch Set 1 #

Patch Set 2 : remove unused function decl #

Total comments: 6

Patch Set 3 : use timeout instead of idle, drop ibus_observed_path_traverse() usage, and use GFile as a key of mo… #

Total comments: 12

Patch Set 4 : remove g_monitor_timeout #

Total comments: 10

Patch Set 5 : improve ibus_observed_path_traverse, etc. #

Total comments: 14

Patch Set 6 : fix the previous CL #

Patch Set 7 : use g_hash_table_lookup instead of g_hash_table_lookup_extended #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -129 lines) Patch
M bus/global.h View 3 5 1 chunk +0 lines, -3 lines 0 comments Download
M bus/global.c View 1 2 3 5 1 chunk +0 lines, -4 lines 0 comments Download
M bus/ibusimpl.c View 1 2 3 5 1 chunk +1 line, -7 lines 0 comments Download
M bus/main.c View 3 5 1 chunk +0 lines, -3 lines 0 comments Download
M bus/registry.h View 1 2 chunks +2 lines, -4 lines 0 comments Download
M bus/registry.c View 1 2 3 4 5 6 6 chunks +86 lines, -94 lines 0 comments Download
M bus/server.c View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M src/ibuscomponent.h View 1 chunk +12 lines, -0 lines 0 comments Download
M src/ibuscomponent.c View 1 2 3 4 5 4 chunks +14 lines, -7 lines 0 comments Download
M src/ibusobservedpath.h View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments Download
M src/ibusobservedpath.c View 1 2 3 4 5 2 chunks +9 lines, -5 lines 0 comments Download

Messages

Total messages: 12
Daiki Ueno
1 year, 6 months ago #1
Peng
https://codereview.appspot.com/6589065/diff/1009/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/1009/bus/registry.c#newcode553 bus/registry.c:553: registry->monitor_idle_id = g_idle_add ((GSourceFunc) _monitor_idle_cb, How about use a ...
1 year, 6 months ago #2
Daiki Ueno
I'm very sorry for not updating this. https://codereview.appspot.com/6589065/diff/1009/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/1009/bus/registry.c#newcode553 bus/registry.c:553: registry->monitor_idle_id = ...
1 year, 6 months ago #3
Peng
https://codereview.appspot.com/6589065/diff/5001/bus/global.c File bus/global.c (right): https://codereview.appspot.com/6589065/diff/5001/bus/global.c#newcode32 bus/global.c:32: gint g_monitor_timeout = 5000; Maybe we should remove it. ...
1 year, 5 months ago #4
Daiki Ueno
https://codereview.appspot.com/6589065/diff/5001/bus/global.c File bus/global.c (right): https://codereview.appspot.com/6589065/diff/5001/bus/global.c#newcode32 bus/global.c:32: gint g_monitor_timeout = 5000; On 2012/10/23 01:44:27, Peng wrote: ...
1 year, 5 months ago #5
Peng
https://codereview.appspot.com/6589065/diff/13001/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/13001/bus/registry.c#newcode106 bus/registry.c:106: NULL, Should we provide free function for GFile here? ...
1 year, 5 months ago #6
Daiki Ueno
https://codereview.appspot.com/6589065/diff/13001/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/13001/bus/registry.c#newcode106 bus/registry.c:106: NULL, On 2012/10/23 15:50:35, Peng wrote: > Should we ...
1 year, 5 months ago #7
Peng
https://codereview.appspot.com/6589065/diff/13001/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/13001/bus/registry.c#newcode106 bus/registry.c:106: NULL, On 2012/10/25 01:51:33, Daiki Ueno wrote: > On ...
1 year, 5 months ago #8
Daiki Ueno
https://codereview.appspot.com/6589065/diff/13001/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/13001/bus/registry.c#newcode106 bus/registry.c:106: NULL, On 2012/10/25 20:37:35, Peng wrote: > On 2012/10/25 ...
1 year, 5 months ago #9
Peng
https://codereview.appspot.com/6589065/diff/17004/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/17004/bus/registry.c#newcode607 bus/registry.c:607: g_object_ref (file), On 2012/10/26 01:35:04, Daiki Ueno wrote: > ...
1 year, 5 months ago #10
Daiki Ueno
https://codereview.appspot.com/6589065/diff/17004/bus/registry.c File bus/registry.c (right): https://codereview.appspot.com/6589065/diff/17004/bus/registry.c#newcode607 bus/registry.c:607: g_object_ref (file), On 2012/10/26 01:43:01, Peng wrote: > On ...
1 year, 5 months ago #11
Peng
1 year, 5 months ago #12
Lgtm with one comment

https://codereview.appspot.com/6589065/diff/17004/bus/registry.c
File bus/registry.c (right):

https://codereview.appspot.com/6589065/diff/17004/bus/registry.c#newcode588
bus/registry.c:588: if (!g_hash_table_lookup_extended (registry->monitor_table,
Please use g_hash table_lookup

https://codereview.appspot.com/6589065/diff/17004/bus/registry.c#newcode616
bus/registry.c:616: g_object_unref (file);
On 2012/10/26 02:08:57, Daiki Ueno wrote:
> Here.
Oops. Did not see it.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5