nsPrimitiveHelpers::ConvertPlatformPlainTextToUnicode returns unitialized value if outUnicodeLen = 0

RESOLVED FIXED

Status

()

Core
Widget
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: timeless, Assigned: smontagu)

Tracking

({coverity})

Trunk
x86
Linux
coverity
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

12 years ago
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 :)
(Assignee)

Comment 1

12 years ago
Created attachment 218658 [details] [diff] [review]
patch
Assignee: general → smontagu
Status: NEW → ASSIGNED
Attachment #218658 - Flags: superreview?(jag)
Attachment #218658 - Flags: review?(timeless)
(Assignee)

Comment 2

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

Comment 3

12 years ago
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.
Attachment #218658 - Flags: superreview?(jag) → superreview-
(Assignee)

Comment 4

12 years ago
Created attachment 218770 [details] [diff] [review]
Patch the right place this time

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.
Attachment #218658 - Attachment is obsolete: true
Attachment #218770 - Flags: superreview?(jag)
Attachment #218770 - Flags: review?(timeless)
Attachment #218658 - Flags: review?(timeless)
(Assignee)

Comment 5

12 years ago
Created attachment 218771 [details] [diff] [review]
Same, with incorrect comment removed

While I was there, removed a comment that has been inaccurate since rev 1.14. Sorry for the bugspam
Attachment #218771 - Flags: superreview?(jag)
Attachment #218771 - Flags: review?(timeless)
(Assignee)

Updated

12 years ago
Attachment #218770 - Attachment is obsolete: true
Attachment #218770 - Flags: superreview?(jag)
Attachment #218770 - Flags: review?(timeless)

Comment 6

12 years ago
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?
Attachment #218771 - Flags: superreview?(jag) → superreview+
(Assignee)

Comment 7

12 years ago
Created attachment 219510 [details] [diff] [review]
Updated to last comment
Attachment #218771 - Attachment is obsolete: true
Attachment #219510 - Flags: review?
Attachment #218771 - Flags: review?(timeless)
(Assignee)

Updated

12 years ago
Attachment #219510 - Flags: review? → review?(timeless)
(Reporter)

Updated

12 years ago
Attachment #219510 - Flags: review?(timeless) → review+
(Assignee)

Comment 8

12 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.