From 4f065492effc346180dd22c7edfa28f3054eb451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 22 Mar 2014 00:11:56 +0100 Subject: [PATCH] Unify the usage of cursors Safer and less error-prone because: * Always checks for null * Checks for sizes * Inits App/Apk lists at known capacity * Properly closes all cursors There are still one or two cursors that are not closed correctly and show things like these: W/CursorWrapperInner(19973): Cursor finalized without prior close() --- src/org/fdroid/fdroid/UpdateService.java | 13 ++-- src/org/fdroid/fdroid/data/ApkProvider.java | 26 ++++--- src/org/fdroid/fdroid/data/AppProvider.java | 43 +++++++----- src/org/fdroid/fdroid/data/DBHelper.java | 73 +++++++++++--------- src/org/fdroid/fdroid/data/RepoProvider.java | 16 +++-- 5 files changed, 102 insertions(+), 69 deletions(-) diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index f22b93281..323ab88fc 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -539,12 +539,15 @@ public class UpdateService extends IntentService implements ProgressListener { int knownIdCount = cursor != null ? cursor.getCount() : 0; List knownIds = new ArrayList(knownIdCount); - if (knownIdCount > 0) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - knownIds.add(cursor.getString(0)); - cursor.moveToNext(); + if (cursor != null) { + if (knownIdCount > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + knownIds.add(cursor.getString(0)); + cursor.moveToNext(); + } } + cursor.close(); } return knownIds; diff --git a/src/org/fdroid/fdroid/data/ApkProvider.java b/src/org/fdroid/fdroid/data/ApkProvider.java index e607a53f4..9ecff515a 100644 --- a/src/org/fdroid/fdroid/data/ApkProvider.java +++ b/src/org/fdroid/fdroid/data/ApkProvider.java @@ -32,12 +32,15 @@ public class ApkProvider extends FDroidProvider { } public static List cursorToList(Cursor cursor) { - List apks = new ArrayList(); + int knownApkCount = cursor != null ? cursor.getCount() : 0; + List apks = new ArrayList(knownApkCount); if (cursor != null) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - apks.add(new Apk(cursor)); - cursor.moveToNext(); + if (knownApkCount > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + apks.add(new Apk(cursor)); + cursor.moveToNext(); + } } cursor.close(); } @@ -64,12 +67,15 @@ public class ApkProvider extends FDroidProvider { ContentResolver resolver = context.getContentResolver(); Uri uri = getContentUri(id, versionCode); Cursor cursor = resolver.query(uri, projection, null, null, null); - if (cursor != null && cursor.getCount() > 0) { - cursor.moveToFirst(); - return new Apk(cursor); - } else { - return null; + Apk apk = null; + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + apk = new Apk(cursor); + } + cursor.close(); } + return apk; } public static List findByApp(Context context, String appId) { diff --git a/src/org/fdroid/fdroid/data/AppProvider.java b/src/org/fdroid/fdroid/data/AppProvider.java index 463b57317..95176fcdb 100644 --- a/src/org/fdroid/fdroid/data/AppProvider.java +++ b/src/org/fdroid/fdroid/data/AppProvider.java @@ -40,12 +40,15 @@ public class AppProvider extends FDroidProvider { } private static List cursorToList(Cursor cursor) { - List apps = new ArrayList(); + int knownAppCount = cursor != null ? cursor.getCount() : 0; + List apps = new ArrayList(knownAppCount); if (cursor != null) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - apps.add(new App(cursor)); - cursor.moveToNext(); + if (knownAppCount > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + apps.add(new App(cursor)); + cursor.moveToNext(); + } } cursor.close(); } @@ -71,16 +74,19 @@ public class AppProvider extends FDroidProvider { Cursor cursor = resolver.query(uri, projection, null, null, null ); Set categorySet = new HashSet(); if (cursor != null) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - String categoriesString = cursor.getString(0); - if (categoriesString != null) { - for( String s : Utils.CommaSeparatedList.make(categoriesString)) { - categorySet.add(s); + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + String categoriesString = cursor.getString(0); + if (categoriesString != null) { + for (String s : Utils.CommaSeparatedList.make(categoriesString)) { + categorySet.add(s); + } } + cursor.moveToNext(); } - cursor.moveToNext(); } + cursor.close(); } List categories = new ArrayList(categorySet); Collections.sort(categories); @@ -103,12 +109,15 @@ public class AppProvider extends FDroidProvider { String[] projection) { Uri uri = getContentUri(appId); Cursor cursor = resolver.query(uri, projection, null, null, null); - if (cursor != null && cursor.getCount() > 0) { - cursor.moveToFirst(); - return new App(cursor); - } else { - return null; + App app = null; + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + app = new App(cursor); + } + cursor.close(); } + return app; } public static void deleteAppsWithNoApks(ContentResolver resolver) { diff --git a/src/org/fdroid/fdroid/data/DBHelper.java b/src/org/fdroid/fdroid/data/DBHelper.java index 3e41c6139..feb323d91 100644 --- a/src/org/fdroid/fdroid/data/DBHelper.java +++ b/src/org/fdroid/fdroid/data/DBHelper.java @@ -102,19 +102,22 @@ public class DBHelper extends SQLiteOpenHelper { String[] columns = { "address", "_id" }; Cursor cursor = db.query(TABLE_REPO, columns, "name IS NULL OR name = ''", null, null, null, null); - if (cursor.getCount() > 0) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - String address = cursor.getString(0); - long id = cursor.getInt(1); - ContentValues values = new ContentValues(1); - String name = Repo.addressToName(address); - values.put("name", name); - String[] args = { Long.toString( id ) }; - Log.i("FDroid", "Setting repo name to '" + name + "' for repo " + address); - db.update(TABLE_REPO, values, "_id = ?", args); - cursor.moveToNext(); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + String address = cursor.getString(0); + long id = cursor.getInt(1); + ContentValues values = new ContentValues(1); + String name = Repo.addressToName(address); + values.put("name", name); + String[] args = { Long.toString( id ) }; + Log.i("FDroid", "Setting repo name to '" + name + "' for repo " + address); + db.update(TABLE_REPO, values, "_id = ?", args); + cursor.moveToNext(); + } } + cursor.close(); } } } @@ -254,19 +257,23 @@ public class DBHelper extends SQLiteOpenHelper { private void migrateRepoTable(SQLiteDatabase db, int oldVersion) { if (oldVersion < 20) { List oldrepos = new ArrayList(); - Cursor c = db.query(TABLE_REPO, + Cursor cursor = db.query(TABLE_REPO, new String[] { "address", "inuse", "pubkey" }, null, null, null, null, null); - c.moveToFirst(); - while (!c.isAfterLast()) { - Repo repo = new Repo(); - repo.address = c.getString(0); - repo.inuse = (c.getInt(1) == 1); - repo.pubkey = c.getString(2); - oldrepos.add(repo); - c.moveToNext(); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + Repo repo = new Repo(); + repo.address = cursor.getString(0); + repo.inuse = (cursor.getInt(1) == 1); + repo.pubkey = cursor.getString(2); + oldrepos.add(repo); + cursor.moveToNext(); + } + } + cursor.close(); } - c.close(); db.execSQL("drop table " + TABLE_REPO); db.execSQL(CREATE_TABLE_REPO); for (Repo repo : oldrepos) { @@ -316,18 +323,22 @@ public class DBHelper extends SQLiteOpenHelper { if (!columnExists(db, TABLE_REPO, "fingerprint")) db.execSQL("alter table " + TABLE_REPO + " add column fingerprint text"); List oldrepos = new ArrayList(); - Cursor c = db.query(TABLE_REPO, + Cursor cursor = db.query(TABLE_REPO, new String[] { "address", "pubkey" }, null, null, null, null, null); - c.moveToFirst(); - while (!c.isAfterLast()) { - Repo repo = new Repo(); - repo.address = c.getString(0); - repo.pubkey = c.getString(1); - oldrepos.add(repo); - c.moveToNext(); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + Repo repo = new Repo(); + repo.address = cursor.getString(0); + repo.pubkey = cursor.getString(1); + oldrepos.add(repo); + cursor.moveToNext(); + } + } + cursor.close(); } - c.close(); for (Repo repo : oldrepos) { ContentValues values = new ContentValues(); values.put("fingerprint", Utils.calcFingerprint(repo.pubkey)); diff --git a/src/org/fdroid/fdroid/data/RepoProvider.java b/src/org/fdroid/fdroid/data/RepoProvider.java index 77e567a4d..5be9fd44b 100644 --- a/src/org/fdroid/fdroid/data/RepoProvider.java +++ b/src/org/fdroid/fdroid/data/RepoProvider.java @@ -33,6 +33,7 @@ public class RepoProvider extends FDroidProvider { if (cursor != null) { cursor.moveToFirst(); repo = new Repo(cursor); + cursor.close(); } return repo; } @@ -176,13 +177,16 @@ public class RepoProvider extends FDroidProvider { ContentResolver resolver = context.getContentResolver(); String[] projection = { ApkProvider.DataColumns._COUNT_DISTINCT_ID }; Uri apkUri = ApkProvider.getRepoUri(repoId); - Cursor result = resolver.query(apkUri, projection, null, null, null); - if (result != null && result.getCount() > 0) { - result.moveToFirst(); - return result.getInt(0); - } else { - return 0; + Cursor cursor = resolver.query(apkUri, projection, null, null, null); + int count = 0; + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + count = cursor.getInt(0); + } + cursor.close(); } + return count; } }