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

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[:aceman]

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.

Comment 1

[:aceman]

Attachment: patch

Comment 2

[[e-mail]]

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.

Comment 3

[:aceman]

Attachment: patch v2

Thanks Neil.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 668905 [details] [diff] [review]
patch v2

This doesn't change interfaces, so it doesn't need sr.

Comment 5

[Mark Banner (:standard8)]

Checked in: https://hg.mozilla.org/comm-central/rev/d1344e39bd90