Last Comment Bug 532108 - Null pointer dereference or redundant check in mailnews/mime/src/mimei.cpp.
: Null pointer dereference or redundant check in mailnews/mime/src/mimei.cpp.
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4
Product: MailNews Core
Classification: Components
Component: MIME (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 3.1a1
Assigned To: Adam Buchbinder
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-01 08:24 PST by Adam Buchbinder
Modified: 2010-03-18 06:41 PDT (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.2-fixed


Attachments
Patch to move the assignment below the check. (757 bytes, patch)
2009-12-31 16:51 PST, Adam Buchbinder
mozilla: review-
Details | Diff | Review
Move the assignment below the check [Checkin: Comment 5 & 7] (737 bytes, patch)
2010-01-04 06:36 PST, Adam Buchbinder
mozilla: review+
standard8: superreview+
standard8: approval‑thunderbird3.0.1-
standard8: approval‑thunderbird3.0.2+
Details | Diff | Review

Description Adam Buchbinder 2009-12-01 08:24:29 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091109 Ubuntu/9.10 (karmic) Firefox/3.5.5
Build Identifier: thunderbird-3.0rc1

cppcheck, a static code analysis tool, reported:

[comm-1.9.1/mailnews/mime/src/mimei.cpp:1502]: (error) Possible null pointer dereference: options - otherwise it is redundant to check if options is null at line 1505

Reproducible: Always




In mime_parse_url_options(), the 'options' argument is a pointer. It's used by calling "options->headers" before being checked with "if (!options)". If it's expected that options could be null, the assignment to default_headers should be moved below the check; if it's can never be null, the check is redundant and can be removed.

Either way, the test as it stands does nothing; if 'options' is null, it will simply lead to a null pointer reference and subsequent crash.

I'm marking this as minor, because I haven't been able to generate an actual crash. Please advise if I should mark this sort of issue with a different severity.
Comment 1 Adam Buchbinder 2009-12-31 16:51:26 PST
Created attachment 419718 [details] [diff] [review]
Patch to move the assignment below the check.

I've used the simplest change; the assignment now occurs after the check, so it's safe against 'options' being null.
Comment 2 David :Bienvenu 2010-01-01 08:31:14 PST
Comment on attachment 419718 [details] [diff] [review]
Patch to move the assignment below the check.

thx for the patch, Adam - mozilla style would be to move the declaration to be with the assignment, instead of splitting them up. r=me with that change.
Comment 3 Adam Buchbinder 2010-01-04 06:36:14 PST
Created attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

Edited the patch so that the assignment is together with the declaration, per Mozilla style.
Comment 4 David :Bienvenu 2010-01-04 06:47:26 PST
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

thx, Adam.
Comment 5 Serge Gautherie (:sgautherie) 2010-01-06 02:26:49 PST
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]


http://hg.mozilla.org/comm-central/rev/3ed8bb7b219b
Comment 6 Mark Banner (:standard8) 2010-01-10 06:41:02 PST
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

We're not seeing any crashers in this area currently. We've already got a significant amount for 3.0.1 so I don't want to add this as well even thought it may be small.
Comment 7 Mark Banner (:standard8) 2010-02-11 13:50:35 PST
Checked in: http://hg.mozilla.org/releases/comm-1.9.1/rev/b3812420fcab

Note You need to log in before you can comment on or make changes to this bug.