Closed Bug 1242436 Opened 4 years ago Closed 4 years ago

[Static Analysis][Logically dead code] In functions PushNotifier::NotifySubscriptionChangeWorkers and PushNotifier::NotifyPushWorkers

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- affected
firefox47 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1349970, CID 1349969)

Attachments

(1 file)

The Static Analysis tool Coverity added that variable ok is assign false an thus determining a logically dead code. 
For example, the below code from NotifySubscriptionChangeWorkers: 

>>  // Parent process, e10s enabled.
>>  bool ok = false;
>>  nsTArray<ContentParent*> contentActors;
>>  ContentParent::GetAll(contentActors);
>>  for (uint32_t i = 0; i < contentActors.Length(); ++i) {
>>    ok &= contentActors[i]->SendPushSubscriptionChange(
>>      PromiseFlatCString(aScope), IPC::Principal(aPrincipal));
>> }
>>  return ok ? NS_OK : NS_ERROR_FAILURE;

regardless of the value from contentActors[i]->SendPushSubscriptionChange, ok will always be false and in the end function will always return NS_ERROR_FAILURE.

I guess that default value for ok should have been true, and in this way if SendPushSubscriptionChange had failed value of ok would change to false, resulting that function would return  NS_ERROR_FAILURE otherwise returns NS_OK.
Comment on attachment 8711640 [details]
MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32277/diff/1-2/
Attachment #8711640 - Attachment description: MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kcambridge@mozilla.com → MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge
Attachment #8711640 - Flags: review?(kcambridge)
Comment on attachment 8711640 [details]
MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge

https://reviewboard.mozilla.org/r/32277/#review28943

Thanks for catching that!
Attachment #8711640 - Flags: review?(kcambridge) → review+
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fd1b82f7fbee
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Target Milestone: mozilla46 → mozilla47
You need to log in before you can comment on or make changes to this bug.