Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(153)

Issue 6541007: Find device-dependent disk stats file, and skip disk stats if not available. (Closed)

Created:
9 years, 10 months ago by Luigi Semenzato
Modified:
9 years, 7 months ago
Reviewers:
petkov
CC:
chromium-os-reviews_chromium.org, petkov, sosa
Visibility:
Public.

Description

Find device-dependent disk stats file, and skip disk stats if not available. Change-Id: I03afb85e3357dd4c2cf5effd98b194c71d77c71d BUG=12171 TEST=unit tested Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=595a968

Patch Set 1 #

Patch Set 2 : Use a (probably) better stat file #

Total comments: 3

Patch Set 3 : Make stat path computation more general #

Total comments: 5

Patch Set 4 : More small changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -12 lines) Patch
M Makefile View 1 chunk +2 lines, -1 line 0 comments Download
M metrics_daemon.h View 2 chunks +2 lines, -2 lines 0 comments Download
M metrics_daemon.cc View 1 2 3 4 chunks +8 lines, -6 lines 0 comments Download
M metrics_daemon_main.cc View 1 2 3 1 chunk +27 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Luigi Semenzato
This makes the daemon work on ARM and makes it behave better for future systems ...
9 years, 10 months ago (2011-02-18 01:02:56 UTC) #1
petkov
http://codereview.chromium.org/6541007/diff/2001/metrics_daemon.cc File metrics_daemon.cc (right): http://codereview.chromium.org/6541007/diff/2001/metrics_daemon.cc#newcode150 metrics_daemon.cc:150: diskstats_path_("") {} no need to initialize std::string to "" ...
9 years, 10 months ago (2011-02-18 04:52:00 UTC) #2
Luigi Semenzato
Thanks. I followed the suggestions, PTAL On 2011/02/18 04:52:00, petkov wrote: > http://codereview.chromium.org/6541007/diff/2001/metrics_daemon.cc > File ...
9 years, 9 months ago (2011-02-25 23:45:07 UTC) #3
petkov
9 years, 9 months ago (2011-02-26 00:26:46 UTC) #4
A few more readability nits, LGTM otherwise.

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon.cc
File metrics_daemon.cc (right):

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon.cc#newcode150
metrics_daemon.cc:150: diskstats_path_() {}
just remove this one -- implicit construction is OK

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon_main.cc
File metrics_daemon_main.cc (right):

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon_main.cc#newco...
metrics_daemon_main.cc:18: int ret;
per style, variables should be declared close to their use. also,
"initialization should be used instead of declaration and assignment"

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon_main.cc#newco...
metrics_daemon_main.cc:24: ret = rootdev(dev_path_cstr, sizeof(dev_path_cstr),
true, true);
int ret = ...

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon_main.cc#newco...
metrics_daemon_main.cc:31: if (dev_path.compare(0, dev_prefix.length(),
dev_prefix) != 0) {
if (base::StartsWithASCII(dev_path, dev_prefix, false))

http://codereview.chromium.org/6541007/diff/6001/metrics_daemon_main.cc#newco...
metrics_daemon_main.cc:38: return stat_path.append(dev_name.append("/stat"));
This is simpler and doesn't need a comment or stat_path variables:

return "/sys/class/block/" + dev_name + "/stat";

Powered by Google App Engine
This is Rietveld 408576698