Closed
Bug 1226580
Opened 9 years ago
Closed 9 years ago
[Static Analysis][Called C++ object pointer is uninitialized] Function LoadImageWithChannel from image/imgLoader.cpp
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
INVALID
| Tracking | Status | |
|---|---|---|
| firefox45 | --- | affected |
People
(Reporter: andi, Assigned: andi)
References
(Blocks 1 open bug)
Details
(Keywords: clang-analyzer)
User Story
The Static Analysis tool Scan-Build added that variable _retval would have been uninitialized.
This would have happened only if new operator from CreateNewProxyForRequest would have failed due to OOM:
>> LOG_SCOPE_WITH_PARAM(gImgLog, "imgLoader::CreateNewProxyForRequest",
>> "imgRequest", aRequest);
>>
>> /* XXX If we move decoding onto separate threads, we should save off the
>> calling thread here and pass it off to |proxyRequest| so that it call
>> proxy calls to |aObserver|.
>> */
>>
>> RefPtr<imgRequestProxy> proxyRequest = new imgRequestProxy();
In order to fix this not so likely issue we should test the result of the allocation and in the case it fails exit the function with error code and do not dereference *_retval. Also i've removed the static cast when dereferencing _retval since it was useless in this case.
Attachments
(2 files, 1 obsolete file)
|
1.67 KB,
patch
|
Details | Diff | Splinter Review | |
|
1.05 KB,
patch
|
Details | Diff | Splinter Review |
The Static Analysis tool Scan-Build added that variable _retval would have been uninitialized. An assert has been added to check this and also the static_cast has been removed.
|
Assignee | |
Comment 1•9 years ago
|
||
|
||
Updated•9 years ago
|
Keywords: clang-analyzer
|
|
||
Updated•9 years ago
|
Blocks: clang-based-analysis
|
|
Assignee | |
Comment 2•9 years ago
|
||
Updated previous patch.
|
|
Assignee | |
Updated•9 years ago
|
User Story: (updated)
|
|
Assignee | |
Updated•9 years ago
|
User Story: (updated)
|
|
Assignee | |
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/30695/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30695/
Attachment #8707431 -
Flags: review?(seth)
|
|
Assignee | |
Updated•9 years ago
|
Attachment #8707431 -
Flags: review?(netzen)
|
||
Comment 4•9 years ago
|
||
Comment on attachment 8707431 [details]
MozReview Request: Bug 1226580 - in case of OOM new fails so exit function with errro code, expected valid pointer. r?seth@mozilla.com
Seth owns this, letting him do review.
Attachment #8707431 -
Flags: review?(netzen)
|
||
Comment 5•9 years ago
|
||
Comment on attachment 8707431 [details]
MozReview Request: Bug 1226580 - in case of OOM new fails so exit function with errro code, expected valid pointer. r?seth@mozilla.com
https://reviewboard.mozilla.org/r/30695/#review33281
r-. There's no actual problem here.
::: image/imgLoader.cpp:1040
(Diff revision 1)
> + NS_ENSURE_ARG_POINTER(proxyRequest.get());
This is not a fallible allocation, so this error condition cannot happen.
Attachment #8707431 -
Flags: review?(seth)
|
|
Assignee | |
Updated•9 years ago
|
Attachment #8707431 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•