[infer] Errors in GeckoProfileDirectories

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcomella, Assigned: enr0n, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 50
Unspecified
Android
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [lang=java][good next bug])

Attachments

(1 attachment, 1 obsolete attachment)

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/GeckoProfileDirectories.java:142: error: NULL_DEREFERENCE
  object returned by parser.getSections() could be null and is dereferenced at line 142

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/GeckoProfileDirectories.java:194: error: NULL_DEREFERENCE
  object returned by parser.getSections() could be null and is dereferenced at line 194

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/GeckoProfileDirectories.java:215: error: NULL_DEREFERENCE
  object returned by parser.getSections() could be null and is dereferenced at line 215
(Assignee)

Comment 1

2 years ago
Hi Michael,

For these would it work to do something like:

if (parser.getSections() != null) {
   // for loop {
         ...
   }
}

Or is this a naive approach?
Flags: needinfo?(michael.l.comella)
Yep, that should work! Given that these methods return an error value when the value is not found, ignoring the for loop if it'd throw a NullPointerException sounds like a valid approach.
Flags: needinfo?(michael.l.comella)
(Assignee)

Comment 3

2 years ago
Created attachment 8763293 [details] [diff] [review]
bug-1261980.patch
Attachment #8763293 - Flags: review?(liuche)
(Assignee)

Comment 4

2 years ago
Created attachment 8766606 [details] [diff] [review]
bug-1261980.patch
Attachment #8763293 - Attachment is obsolete: true
Attachment #8763293 - Flags: review?(liuche)
Attachment #8766606 - Flags: review?(michael.l.comella)
Comment on attachment 8766606 [details] [diff] [review]
bug-1261980.patch

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

Nice!

I made a push to our try test servers (above).

Once the push goes green, you can add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches added via checkin-needed keyword need an associated green try run. Let me know if you need help reading the results.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8766606 - Flags: review?(michael.l.comella) → review+
Assignee: nobody → nrosbrook
(Assignee)

Comment 7

2 years ago
Michael, could you help me understand the results? I see some robocop failures, but I can't tell if they're from what I did.
Flags: needinfo?(michael.l.comella)
Our test suites often get intermittent failures and these look like some of them. To check, you can see if the suite has been re-run (i.e. there is another test suite with the same name on the same device) and if that was green. Additionally, if you click a failing test suite, you can look at the bar on the bottom of the screen and see if it matches any existing known intermittent failures, which are also listed on that bar. rc4 looks like it failed twice in a row but both with some common errors.

This looks good to me and is ready for check-in!

Let me know if you have more questions!
Flags: needinfo?(michael.l.comella)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/717e8cdc7be7
Added checks in GeckoProfileDirectories to prevent null dereferencing. r=mcomella
Keywords: checkin-needed

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/717e8cdc7be7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.