The Static Analysis tool Coverity added that a variable |c| can cause a null pointer dereference in the following context: >> final Cursor c = readingListAccessor.getReadingListUnfetched(context.getContentResolver()); >> try { >> while (c.moveToNext()) { >> JSONObject json = new JSONObject(); >> try { >> json.put("id", c.getInt(c.getColumnIndexOrThrow(ReadingListItems._ID))); >> json.put("url", c.getString(c.getColumnIndexOrThrow(ReadingListItems.URL))); >> GeckoAppShell.sendEventToGecko( >> GeckoEvent.createBroadcastEvent("Reader:FetchContent", json.toString())); >> } catch (JSONException e) { As |readingListAccessor| is an instance of LocalReadingListAccessor function getReadingListUnfetched calls a query that it's return is NullAble: >> public final @Nullable Cursor query(@NonNull Uri uri, @Nullable String[] projection, >> @Nullable String selection, @Nullable String[] selectionArgs, >> @Nullable String sortOrder) { >> return query(uri, projection, selection, selectionArgs, sortOrder, null); >> } thus |c| can be null

Review commit: https://reviewboard.mozilla.org/r/37127/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37127/

Comment on attachment 8724728 [details] MozReview Request: Bug 1252082 - prevent null pointer dereference on |c|. r?sebastian https://reviewboard.mozilla.org/r/37127/#review33697 ::: mobile/android/base/java/org/mozilla/gecko/ReadingListHelper.java:254 (Diff revision 1) > + if ( c != null ) { NIT: I'd prefer an early return here (cursor == null) instead of wrapping the whole block. Also: We usually do not add spaces around the expression: if (cursor != null).

Comment on attachment 8724728 [details] MozReview Request: Bug 1252082 - prevent null pointer dereference on |c|. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37127/diff/1-2/