Last Comment Bug 745287 - Fix recent a11y build warnings
: Fix recent a11y build warnings
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla14
Assigned To: :Ms2ger (⌚ UTC+1/+2)
: alexander :surkov
Depends on:
Blocks: buildwarning 652635 740758
  Show dependency treegraph
Reported: 2012-04-13 12:09 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2012-04-15 04:38 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (1.89 KB, patch)
2012-04-13 12:09 PDT, :Ms2ger (⌚ UTC+1/+2)
markcapella: review+
dbolter: review+
Details | Diff | Splinter Review

Description User image :Ms2ger (⌚ UTC+1/+2) 2012-04-13 12:09:00 PDT
Created attachment 614873 [details] [diff] [review]
Patch v1
Comment 1 User image David Bolter [:davidb] 2012-04-13 12:20:05 PDT
Comment on attachment 614873 [details] [diff] [review]
Patch v1

Review of attachment 614873 [details] [diff] [review]:

Looks fine to me.
Comment 2 User image Trevor Saunders (:tbsaunde) 2012-04-13 13:38:31 PDT
Comment on attachment 614873 [details] [diff] [review]
Patch v1

>   nsresult rv = GetMaximumValue(&maxValue);
>-  if (maxValue == 0)
>+  if (NS_FAILED(rv) || maxValue == 0)
>     return;

I'm fine with this since I wasn't completely sure I wanted to warn there, but that change isn't quiet equivelent.

On the other hand I tend to think we should either tell gcc to shut up about this warning, or implement a one arg version of the macro as iirc Neil suggested.
Comment 3 User image alexander :surkov 2012-04-13 20:09:37 PDT
yep, I think the change is ok. We don't need to warn about incorrect markup.

Mark, please do a review.
Comment 4 User image :Ms2ger (⌚ UTC+1/+2) 2012-04-15 04:38:50 PDT

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