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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 5 months ago by Daiki Ueno
Modified:
11 years, 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
11 years, 5 months ago (2012-10-03 07:41:03 UTC) #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 ...
11 years, 5 months ago (2012-10-03 13:12:53 UTC) #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 = ...
11 years, 5 months ago (2012-10-19 08:17:10 UTC) #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. ...
11 years, 5 months ago (2012-10-23 01:44:27 UTC) #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: ...
11 years, 5 months ago (2012-10-23 09:23:02 UTC) #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? ...
11 years, 5 months ago (2012-10-23 15:50:33 UTC) #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 ...
11 years, 5 months ago (2012-10-25 01:51:33 UTC) #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 ...
11 years, 5 months ago (2012-10-25 20:37:35 UTC) #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 ...
11 years, 5 months ago (2012-10-26 01:35:03 UTC) #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: > ...
11 years, 5 months ago (2012-10-26 01:43:00 UTC) #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 ...
11 years, 5 months ago (2012-10-26 02:08:57 UTC) #11
Peng
11 years, 5 months ago (2012-10-26 02:37:41 UTC) #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 f62528b