I believe this is a regression from the last check-in for bug 372636. Now I get this: d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp: In member function `HFONT __* gfxWindowsFont::MakeHFONT()': d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:232: warning: comparison b etween signed and unsigned integer expressions d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp: In function `PRBool IsMis singGlyphsGDI(HDC__*, const char*, PRUint32, WCHAR*)': d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:647: error: invalid conver sion from `WCHAR*' to `WORD*' d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp: In function `PRBool IsMis singGlyphsGDI(HDC__*, const PRUnichar*, PRUint32, WCHAR*)': d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:664: error: invalid conver sion from `WCHAR*' to `WORD*' d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp: In constructor `Uniscribe Item::UniscribeItem(gfxContext*, HDC__*, const PRUnichar*, PRUint32, SCRIPT_ITEM *, gfxWindowsFontGroup*)': d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:1425: warning: `UniscribeI tem::mAlternativeString' will be initialized after d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:1419: warning: `SCRIPT_I TEM*UniscribeItem::mScriptItem' d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:901: warning: when initi alized here d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:1434: warning: `UniscribeI tem::mNumGlyphs' will be initialized after d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:1433: warning: `int Unis cribeItem::mMaxGlyphs' d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:901: warning: when initi alized here d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp: In member function `void gfxWindowsFontGroup::InitTextRunUniscribe(gfxContext*, gfxTextRun*, const PRUnic har*, PRUint32)': d:/mozilla/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp:1537: warning: unused vari able 'fontIndex' ../../../dist/include/thebes/gfxWindowsFonts.h: At global scope: ../../../dist/include/thebes/gfxWindowsFonts.h:232: warning: 'PRUint8 CharRangeB it(PRUint32)' defined but not used make[6]: *** [gfxWindowsFonts.o] Error 1 make[6]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw/gfx/thebes/ src' make[5]: *** [libs] Error 2 make[5]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw/gfx/thebes' make[4]: *** [libs] Error 2 make[4]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw/gfx' make[3]: *** [libs_tier_gecko] Error 2 make[3]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw' make[2]: *** [tier_gecko] Error 2 make[2]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw' make[1]: *** [alldep] Error 2 make[1]: Leaving directory `/cygdrive/d/mozilla/mozilla/object-mingw' make: *** [alldep] Error 2 To make sure I backed out one step on gfxWindowsFonts.cpp (1.88 -> 1.87) and then it built just fine.

This seems to fix it for me. Btw, I don't seem to need the patch for bug 367742 anymore, at least I don't trip anymore over a compiler error without that patch.

Yes, Martijn. Your patch works for me too. I simply took the current version (1.88) of gfxWindowsFonts.cpp and applied your patch manually. I also made a build with VC8, to check that also this worked, and it did. So, can we please have this reviewed and checked in?

Checking in gfxWindowsFonts.cpp; /cvsroot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp,v <-- gfxWindowsFonts.cpp new revision: 1.89; previous revision: 1.88 done Checked in on trunk.

I think this patch is bad example of programming. Consider the use of function 'IsMissingGlyphsGDI'. InitTextRunGDI allocates on stack sizeof(WCHAR) * 500 bytes (why 500 and why use nsAutoTArray when WCHAR glyphArray[500] will be enough?) using nsAutoTArray and immediately reserves sizeof(WCHAR) * aLength bytes from it. Next auto array pointer is passed to IsMissingGlyphsGDI, and there it is passed to GetGlyphIndicesA which expects (according to documentation) array of length aLength of WORDs (indices to glyphs array) - not WCHARS (characters). Of course sizeof(WCHAR)==sizeof(WORD) and we do not overwrite memory here. Furthermore, few lines after the call to IsMissingGlyphsGDI in InitTextRunGDI the same buffer is reused for other purpose, and hence value set in IsMissingGlyphsGDI isn't used anywhere except in IsMissingGlyphsGDI. So it is possible to exclude one argument from arglist of IsMissingGlyphsGDI and allocate this buffer in IsMissingGlyphsGDI itself without confusing someone who will look at this code sometime later. For instance my first thought was that after return of IsMissingGlyphsGDI glyphArray values set there are used somehow, but only return value of this function is used.

Serge, you're more than welcome to write a better patch. Thanks!

I don't think the patch as such is an example of bad programming. But maybe, considering the large number of compiler warnings, with gcc, with mingw, with VC8, that indicates that the whole of gfxWindowsFonts.cpp should be looked at. There are certainly room for improvements, I think everyone agrees to that. But the main issue to get Mingw happy and at the same time not busting anything else, other compilers, other platforms etc is good. Thanks Martijn for the quick action!

I keep trying to fix the warnings but they keep coming back.. :/