1013263 - Notification::CloseInternal: warning: ‘rv’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nathan Froyd [:froydnj]]

This warning is completely legitimate:

    nsresult rv;
    // Don't bail out if notification storage fails, since we still
    // want to send the close event through the alert service.
    nsCOMPtr<nsINotificationStorage> notificationStorage =
      do_GetService(NS_NOTIFICATION_STORAGE_CONTRACTID);
    if (notificationStorage) {
      nsString origin;
      rv = GetOrigin(GetOwner(), origin);
      if (NS_SUCCEEDED(rv)) {
        notificationStorage->Delete(origin, mID);
      }
    }

    nsCOMPtr<nsIAlertsService> alertService =
      do_GetService(NS_ALERTSERVICE_CONTRACTID);
    if (alertService) {
      if (NS_SUCCEEDED(rv)) { // This is the uninitialized use.
        alertService->CloseAlert(mAlertName, GetPrincipal());
      }
    }

The first comment suggests that we want to always send the alert, so I guess we should just delete the NS_SUCCEEDED check?

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: fix -Wmaybe-uninitialized warning in Notification.cpp

Given the initial comment about always wanting to send the close event, we
shouldn't need the NS_SUCCEEDED check.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: fix -Wmaybe-uninitialized warning in Notification.cpp

Here, have a version of the patch without a silly typo.

Comment 3

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8426399 [details] [diff] [review]
fix -Wmaybe-uninitialized warning in Notification.cpp

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

Good catch! Thanks.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1dfe3f782b41