Last Comment Bug 739635 - fix -Wunused-but-set-variable warnings in gfx/thebes/
: fix -Wunused-but-set-variable warnings in gfx/thebes/
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: mozilla14
Assigned To: Nobody; OK to take it and work on it
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2012-03-27 08:56 PDT by Nathan Froyd [:froydnj]
Modified: 2012-03-29 08:53 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.00 KB, patch)
2012-03-27 08:57 PDT, Nathan Froyd [:froydnj]
no flags Details | Diff | Splinter Review
patch (4.41 KB, patch)
2012-03-28 07:55 PDT, Nathan Froyd [:froydnj]
jmuizelaar: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-03-27 08:56:15 PDT
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp: In member function 'gfxShapedWord* gfxFont::GetShapedWord(gfxContext*, const T*, PRUint32, PRUint32, PRInt32, PRInt32, PRUint32) [with T = unsigned char, PRUint32 = unsigned int, PRInt32 = int]':
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:5260:83:   instantiated from here
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:2044:10: warning: variable 'ok' set but not used [-Wunused-but-set-variable]
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp: In member function 'gfxShapedWord* gfxFont::GetShapedWord(gfxContext*, const T*, PRUint32, PRUint32, PRInt32, PRInt32, PRUint32) [with T = short unsigned int, PRUint32 = unsigned int, PRInt32 = int]':
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:2241:56:   instantiated from 'bool gfxFont::SplitAndInitTextRun(gfxContext*, gfxTextRun*, const T*, PRUint32, PRUint32, PRInt32) [with T = short unsigned int, PRUint32 = unsigned int, PRInt32 = int]'
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:3381:13:   instantiated from 'void gfxFontGroup::InitScriptRun(gfxContext*, gfxTextRun*, const T*, PRUint32, PRUint32, PRInt32) [with T = short unsigned int, PRUint32 = unsigned int, PRInt32 = int]'
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:3328:13:   instantiated from 'void gfxFontGroup::InitTextRun(gfxContext*, gfxTextRun*, const T*, PRUint32) [with T = unsigned char, PRUint32 = unsigned int]'
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:3206:61:   instantiated from here
/home/froydnj/src/m-c.git/gfx/thebes/gfxFont.cpp:2044:10: warning: variable 'ok' set but not used [-Wunused-but-set-variable]
/home/froydnj/src/m-c.git/gfx/thebes/gfxFontUtils.cpp: In static member function 'static nsresult gfxFontUtils::RenameFont(const nsAString_internal&, const PRUint8*, PRUint32, FallibleTArray<unsigned char>*)':
/home/froydnj/src/m-c.git/gfx/thebes/gfxFontUtils.cpp:1426:10: warning: variable 'foundName' set but not used [-Wunused-but-set-variable]
Comment 1 Nathan Froyd [:froydnj] 2012-03-27 08:57:22 PDT
Created attachment 609739 [details] [diff] [review]
patch
Comment 2 Jonathan Kew (:jfkthame) 2012-03-27 15:11:19 PDT
Comment on attachment 609739 [details] [diff] [review]
patch

Review of attachment 609739 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxFontUtils.cpp
@@ +1422,5 @@
>      TableDirEntry *dirEntry = 
>          reinterpret_cast<TableDirEntry*>(newFontData + sizeof(SFNTHeader));
>  
>      PRUint32 numTables = sfntHeader->numTables;
> +    DebugOnly<bool> foundName = false;

Rather than making this DebugOnly, I'd suggest removing it altogether and changing the NS_ASSERTION that used it to assert that i < numTables instead.
Comment 3 Nathan Froyd [:froydnj] 2012-03-28 07:55:08 PDT
Created attachment 610135 [details] [diff] [review]
patch

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> >      PRUint32 numTables = sfntHeader->numTables;
> > +    DebugOnly<bool> foundName = false;
> 
> Rather than making this DebugOnly, I'd suggest removing it altogether and
> changing the NS_ASSERTION that used it to assert that i < numTables instead.

Thanks, done.  I also took care of the warnings about foundOS2 and nameLen; nameLen seemed rather easy, but I wasn't sure of the best way to deal with foundOS2, since it only applies for XP_WIN.
Comment 4 Jeff Muizelaar [:jrmuizel] 2012-03-28 13:16:14 PDT
Comment on attachment 610135 [details] [diff] [review]
patch

Review of attachment 610135 [details] [diff] [review]:
-----------------------------------------------------------------

Sure.
Comment 6 Matt Brubeck (:mbrubeck) 2012-03-29 08:53:41 PDT
https://hg.mozilla.org/mozilla-central/rev/378f53635589

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