660743 - storage should use mozilla::Preferences

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 536226 [details] [diff] [review]
Patch v1.0

Review of attachment 536226 [details] [diff] [review]:
-----------------------------------------------------------------

::: storage/src/VacuumManager.cpp
@@ +271,4 @@
>    // Check interval from last vacuum.
>    PRInt32 now = static_cast<PRInt32>(PR_Now() / PR_USEC_PER_SEC);
>    PRInt32 lastVacuum;
> +  nsPrintfCString prefName("%s%s", PREF_VACUUM_BRANCH, mDBFilename.get());

nsPrintfCString says
   * This class is intended to be useful for numbers and short
   * strings, not arbitrary formatting of other strings (e.g., with %s).

I think you should use an nsAutoCString and Append.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.1

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 536237 [details] [diff] [review]
Patch v1.1

Review of attachment 536237 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 4

[Shawn Wilsher :sdwilsh]

Comment on attachment 536237 [details] [diff] [review]
Patch v1.1

Review of attachment 536237 [details] [diff] [review]:
-----------------------------------------------------------------

r=sdwilsh with comments addressed.

::: storage/src/VacuumManager.cpp
@@ +444,1 @@
>  VacuumManager::initialize()

There is no longer a need for this method, so it can disappear.  Please make it so.

@@ +484,1 @@
>        NS_ENSURE_STATE(vacuum);

nix this check since you are here; new is infallible now.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://hg.mozilla.org/mozilla-central/rev/82a5618aa8c6