it's a rare case, in fact, it's probably virtually impossible. but the code should probably favor initializing rv to NS_OK and i wonder if GetMaxLength can fail :)

In fact I don't know how impossible this is: I don't see anything stopping inTextLen from being 0...

Comment on attachment 218658 [details] [diff] [review] patch This seems like the wrong patch. That rv is pretty much directly used in a do_GetService(..., &rv) which will always set it to something.

You're right, I patched the wrong place, but do you have any objection to initializing both rvs to NS_OK? It seems like best practice to me.

While I was there, removed a comment that has been inaccurate since rev 1.14. Sorry for the bugspam

Comment on attachment 218771 [details] [diff] [review] Same, with incorrect comment removed I guess I would reorganize the code to make it more clear that rv gets properly initialized, something along the lines of: // get the charset nsresult rv; nsCOMPtr<nsIPlatformCharset> platformCharsetService = do_GetService(NS_PLATFORMCHARSET_CONTRACTID, &rv); nsCAutoString platformCharset; if (NS_SUCCEEDED(rv)) rv = platformCharsetService->GetCharset(kPlatformCharsetSel_PlainTextInClipboard, platformCharset); if (NS_FAILED(rv)) platformCharset.AssignLiteral("ISO-8859-1"); Btw, am I missing something, or is |encoder| completely unused?

Checked in.