Closed Bug 1459470 Opened Last year Closed Last year

Replace NS_PRECONDITION with NS_ASSERTION or MOZ_ASSERT

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 61.0

People

(Reporter: cpeterson, Assigned: jorgk)

References

Details

Attachments

(1 file, 1 obsolete file)

I will be removing the NS_PRECONDITION assertion macro in Firefox Nightly 62 next week in Gecko bug 1457813. NS_PRECONDITION is just an alias for NS_ASSERTION.

* For NS_PRECONDITION calls that do not assert in Firefox's Try tests, I will replace them with fatal MOZ_ASSERT calls.

* For the three NS_PRECONDITION calls that do assert, I will replace them with non-fatal NS_ASSERTION.

Thunderbird will need either:

* Replace its NS_PRECONDITION calls with NS_ASSERTION (or MOZ_ASSERT if you are sure the assertions do not current fail)

* Or add back the NS_PRECONDITION definition by reverting this patch:

https://reviewboard.mozilla.org/r/240678/diff/1#index_header

It looks like there are 107 NS_PRECONDITION calls in comm-central's ldap and mailnews directories:

https://searchfox.org/comm-central/search?q=NS_PRECONDITION
Thanks for the heads-up!
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Boring review. I used sed and then went through and fixed some indentation issues manually. There were sadly also lots of other indentation issues in the vicinity, so I fixed them, too.
Attachment #8973549 - Flags: review?(acelists)
Comment on attachment 8973549 [details] [diff] [review]
1459470-replace-NS_PRECONDITION.patch

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

Thanks.

::: mailnews/import/outlook/src/nsOutlookImport.cpp
@@ +494,5 @@
>    m_msgCount = 0;
>    m_msgTotal = 0;
> +  NS_ASSERTION(source != nullptr, "null ptr");
> +  NS_ASSERTION(destination != nullptr, "null ptr");
> +  NS_ASSERTION(fatalError != nullptr, "null ptr");

Thanks for fixing the indent.

::: mailnews/import/src/nsImportAddressBooks.cpp
@@ +115,5 @@
>  
>  
>  nsresult NS_NewGenericAddressBooks(nsIImportGeneric** aImportGeneric)
>  {
> +    NS_ASSERTION(aImportGeneric != nullptr, "null ptr");

Bad indent preserved :)

@@ +639,5 @@
>  }
>  
>  NS_IMETHODIMP nsImportGenericAddressBooks::ContinueImport(bool *_retval)
>  {
> +    NS_ASSERTION(_retval != nullptr, "null ptr");

Bad indent preserved.

@@ +657,5 @@
>  NS_IMETHODIMP nsImportGenericAddressBooks::GetProgress(int32_t *_retval)
>  {
>    // This returns the progress from the the currently
>    // running import mail or import address book thread.
> +    NS_ASSERTION(_retval != nullptr, "null ptr");

Bad indent preserved.

::: mailnews/import/src/nsImportMail.cpp
@@ +144,5 @@
>  
>  nsresult NS_NewGenericMail(nsIImportGeneric** aImportGeneric)
>  {
> +  NS_ASSERTION(aImportGeneric != nullptr, "null ptr");
> +  if (! aImportGeneric)

You could remove the space when touching this.

@@ +815,5 @@
>    }
>  
>    // Now save the new acct info to pref file.
>    nsCOMPtr <nsIMsgAccountManager> accMgr = do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);
> +  if (NS_SUCCEEDED(rv) && accMgr) {

Thanks for fixing the indent.

::: mailnews/import/src/nsImportService.cpp
@@ +96,5 @@
>  
>  NS_IMETHODIMP nsImportService::CreateNewGenericMail(nsIImportGeneric **_retval)
>  {
> +  NS_ASSERTION(_retval != nullptr, "null ptr");
> +  if (! _retval)

Remove the space.

@@ +107,5 @@
>  
>  NS_IMETHODIMP nsImportService::CreateNewGenericAddressBooks(nsIImportGeneric **_retval)
>  {
> +  NS_ASSERTION(_retval != nullptr, "null ptr");
> +  if (! _retval)

Remove the space.

@@ +117,5 @@
>  
>  NS_IMETHODIMP nsImportService::GetModuleCount(const char *filter, int32_t *_retval)
>  {
> +  NS_ASSERTION(_retval != nullptr, "null ptr");
> +  if (! _retval)

Remove the space.
Attachment #8973549 - Flags: review?(acelists) → review+
Fixed more white-space issues.
Attachment #8973549 - Attachment is obsolete: true
Attachment #8973557 - Flags: review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/58409b0e6640
Replace NS_PRECONDITION with NS_ASSERTION after removal in bug 1457813. r=aceman
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 61.0
See Also: → 1470374
You need to log in before you can comment on or make changes to this bug.