Closed Bug 550962 Opened 10 years ago Closed 10 years ago

add coverity hint in CSSParserImpl to ignore return values from ParseEnum/ExpectSymbol

Categories

(Core :: CSS Parsing and Computation, defect, minor)

x86
All
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 3 obsolete files)

zwol indicates that these are intentional but that he'd be ok w/ adding (void) for them.

6815 CSSParserImpl::ParseBorderImage()
6884   if (ParseEnum(horizontalKeyword, nsCSSProps::kBorderImageKTable)) {
6885     ParseEnum(verticalKeyword, nsCSSProps::kBorderImageKTable);

7134 CSSParserImpl::DoParseRect(nsCSSRect& aRect)
7171       if (3 != side) {
7172         // skip optional commas between elements
7173         ExpectSymbol(',', PR_TRUE);
dbaron, do you have an opinion?
Attached patch proposal (obsolete) — Splinter Review
timeless-mbp:mozilla-central timeless$ grep '(void)' layout/style/nsComputedDOMStyle.cpp
  (void)GetQueryablePropertyMap(aLength);

So, this matches that style.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #431164 - Flags: review?(zweinberg)
Comment on attachment 431164 [details] [diff] [review]
proposal

This changes ParseBorderImage and DoParseRect.  Weren't you going to change ParseDasharray?

(FWIW, this appears to be a correct change in DoParseRect as well.)
Attached patch proposal (obsolete) — Splinter Review
Attachment #431164 - Attachment is obsolete: true
Attachment #431207 - Flags: review?(zweinberg)
Attachment #431164 - Flags: review?(zweinberg)
Attached patch proposal (obsolete) — Splinter Review
Attachment #431207 - Attachment is obsolete: true
Attachment #431210 - Flags: review?(zweinberg)
Attachment #431207 - Flags: review?(zweinberg)
Comment on attachment 431210 [details] [diff] [review]
proposal

Looks good.

>@@ -9084,7 +9084,7 @@ CSSParserImpl::ParseDasharray()
>         break;
> 
>        // skip optional commas between elements
>-      ExpectSymbol(',', PR_TRUE);
>+      (void)ExpectSymbol(',', PR_TRUE);
> 
>       if (!ParseVariant(value,
>                         VARIANT_LENGTH | VARIANT_PERCENT | VARIANT_NUMBER,

Would you mind fixing the indentation of the comment here, as long as you're in the file?  (It should be aligned with the code, not one space further in.)
Attachment #431210 - Flags: review?(zweinberg) → review+
Attached patch align commentSplinter Review
Attachment #431210 - Attachment is obsolete: true
Attachment #431308 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/e2eca12381c4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.