[infer] Errors in FilePickerResultHandler

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 48
All
Android
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(2 attachments)

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:101: error: NULL_DEREFERENCE
  object cursor last assigned on line 99 could be null and is dereferenced at line 101

--
/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:238: error: RESOURCE_LEAK
   resource acquired to fos by call to FileOutputStream(...) at line 222 is not released after line 238
Created attachment 8737955 [details]
MozReview Request: Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

Review commit: https://reviewboard.mozilla.org/r/44205/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44205/
Attachment #8737955 - Flags: review?(gkruglov)
Attachment #8737956 - Flags: review?(gkruglov)
Created attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

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

Comment 3

3 years ago
Comment on attachment 8737955 [details]
MozReview Request: Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44205/#review41001

Looks good

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:198
(Diff revision 1)
>                                      null); // sortOrder
>          }
>  
>          @Override
>          public void onLoadFinished(Loader<Cursor> loader, Cursor cursor) {
>              if (cursor.moveToFirst()) {

Since you're touching this, maybe short-circuit out of the function here?
Attachment #8737955 - Flags: review?(gkruglov) → review+

Comment 4

3 years ago
Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44207/#review41005

looks good other than little typo

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:97
(Diff revision 1)
>          final FragmentActivity fa = (FragmentActivity) GeckoAppShell.getGeckoInterface().getActivity();
>          final LoaderManager lm = fa.getSupportLoaderManager();
> +
>          // Finally, Video pickers and some file pickers may return a content provider.
> -        Cursor cursor = null;
> +        final ContentResolver cr = fa.getContentResolver();
> +        final Cursor cursor = cr.query(uri, new String[] { MediaStore.Video.Media.DATA }, null, null, null);;

two semicolons at the end of the line here
Attachment #8737956 - Flags: review?(gkruglov)
https://reviewboard.mozilla.org/r/44205/#review41001

> Since you're touching this, maybe short-circuit out of the function here?

If I was making more drastic changes, I'd consider it but it'll change the indentations and thus the commit history so I don't think it's worth it.
https://reviewboard.mozilla.org/r/44207/#review41005

> two semicolons at the end of the line here

Who would have thought? It compiles with two semi-colons! (...but I guess that makes sense)
Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44207/diff/1-2/
Attachment #8737956 - Flags: review?(gkruglov)

Comment 8

3 years ago
Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44207/#review41789

+1
Attachment #8737956 - Flags: review?(gkruglov) → review+
https://hg.mozilla.org/integration/fx-team/rev/18434342c5638551756e00bec2463c30a4a9abcc
Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

https://hg.mozilla.org/integration/fx-team/rev/81f58f4715ac5ec76ee7514ac172c2c66b1df6e1
Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/18434342c563
https://hg.mozilla.org/mozilla-central/rev/81f58f4715ac
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.