The default bug view has changed. See this FAQ.

fix "warning: overflow in implicit constant conversion [-Woverflow]" in mailnews/local/src/nsParseMailbox.cpp

RESOLVED FIXED in Thunderbird 18.0

Status

MailNews Core
Database
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 18.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

14.53 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
I get these warnings with gcc 4.7 on linux:
mailnews/local/src/nsParseMailbox.cpp: In member function 'virtual int32_t nsMsgMailboxParser::HandleLine(char*, uint32_t)':
mailnews/local/src/nsParseMailbox.cpp:492:18: warning: overflow in implicit constant conversion [-Woverflow]
mailnews/local/src/nsParseMailbox.cpp: In member function 'int nsParseMailMessageState::FinalizeHeaders()':
mailnews/local/src/nsParseMailbox.cpp:1668:16: warning: overflow in implicit constant conversion [-Woverflow]

I convert the mentioned functions to nsresult which causes the need of other called functions to be converted to nsresult too.
(Assignee)

Comment 1

5 years ago
Created attachment 668863 [details] [diff] [review]
patch
Attachment #668863 - Flags: superreview?(mbanner)
Attachment #668863 - Flags: review?(neil)

Comment 2

5 years ago
Comment on attachment 668863 [details] [diff] [review]
patch

>   if (line[0] == 'F' && IsEnvelopeLine(line, lineLength))
...
>     OnNewMessage(nullptr);
>-    status = StartNewEnvelope(line, lineLength);
>-    NS_ASSERTION(status >= 0, " error starting envelope parsing mailbox");
>+    rv = StartNewEnvelope(line, lineLength);
>+    NS_ASSERTION(NS_SUCCEEDED(rv), " error starting envelope parsing mailbox");
>     // at the start of each new message, update the progress bar
>     UpdateProgressPercent();
>-    if (status < 0)
>-      return status;
>+    NS_ENSURE_SUCCESS(rv, rv);
>   }
>   // otherwise, the message parser can handle it completely.
>-  else if (m_mailDB != nullptr)  // if no DB, do we need to parse at all?
>-    return ParseFolderLine(line, lineLength);
>-        else
>-          return NS_ERROR_NULL_POINTER; // need to error out if we don't have a db.
>-
>-  return 0;
>-
>+  else
>+  {
>+    if (m_mailDB != nullptr)  // if no DB, do we need to parse at all?
>+      return ParseFolderLine(line, lineLength);
>+    else
>+      return NS_ERROR_NULL_POINTER; // need to error out if we don't have a db.
>+  }
>+  return NS_OK;
This was badly written, yet your changes make it even worse. The original code looks like this:

if (line[0] == 'F' && IsEnvelopeLine(line, lineLength))
{
  ...
  status = StartNewEnvelope(line, lineLength);
  ...
  if (status < 0)
    return status;
}
else if (m_mailDB != nullptr)  // if no DB, do we need to parse at all?
  return ParseFolderLine(line, lineLength);
else
  return NS_ERROR_NULL_POINTER; // need to error out if we don't have a db.
return 0;

As you can see, the return 0 is only reached if the line is an envelope line, so you can move it into the if block, thus eliminating the inner condition. Having done this, the subsequent else clauses also become unnecessary since control can no longer reach there. The code therefore ends up something like this:

if (line[0] == 'F' && IsEnvelopeLine(line, lineLength))
{
  ...
  nsresult rv = StartNewEnvelope(line, lineLength);
  ...
  return rv;
}

if (m_mailDB != nullptr) // if no DB, do we need to parse at all?
  return ParseFolderLine(line, lineLength);

return NS_ERROR_NULL_POINTER; // need to error out if we don't have a db.

r=me with that fixed.
Attachment #668863 - Flags: review?(neil) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 668905 [details] [diff] [review]
patch v2

Thanks Neil.
Attachment #668863 - Attachment is obsolete: true
Attachment #668863 - Flags: superreview?(mbanner)
Attachment #668905 - Flags: superreview?(mbanner)
Comment on attachment 668905 [details] [diff] [review]
patch v2

This doesn't change interfaces, so it doesn't need sr.
Attachment #668905 - Flags: superreview?(mbanner)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
Checked in: https://hg.mozilla.org/comm-central/rev/d1344e39bd90
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
You need to log in before you can comment on or make changes to this bug.