Fix Coverity reports. (Mostly use of uninitialised values.)

Fix Coverity reports.  (Mostly use of uninitialised values.)

CID=537,103419,103631,103632,103633

Committed: https://code.google.com/p/skia/source/detail?r=3522

[bsalomon, 2012-03-27 20:53:24]

https://codereview.appspot.com/5936047/diff/1/include/core/SkPoint.h
File include/core/SkPoint.h (right):

https://codereview.appspot.com/5936047/diff/1/include/core/SkPoint.h#newcode151
include/core/SkPoint.h:151: bool isZero() const { return (fX == 0) || (fY == 0);
}
&&?

https://codereview.appspot.com/5936047/diff/1/src/gpu/GrAAConvexPathRenderer.cpp
File src/gpu/GrAAConvexPathRenderer.cpp (right):

https://codereview.appspot.com/5936047/diff/1/src/gpu/GrAAConvexPathRenderer....
src/gpu/GrAAConvexPathRenderer.cpp:60: p0.setZero();  // Suppress Coverity
warning.
Personally I'd prefer SkPoint p0 = {0, 0}; If we're going to have a zeroing
function naming it reset() would be consistent with matrix and other types.

[reed1, 2012-03-27 20:54:58]

https://codereview.appspot.com/5936047/diff/1/src/ports/SkFontHost_FreeType.cpp
File src/ports/SkFontHost_FreeType.cpp (left):

https://codereview.appspot.com/5936047/diff/1/src/ports/SkFontHost_FreeType.c...
src/ports/SkFontHost_FreeType.cpp:1120: getBBoxForCurrentGlyph(glyph, &bbox);
:( this change seems too bad, as (afaik) FT guarantees to fill out bbox for us.
Do we know that there is a code-path where bbox is uninitialized?

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp
File src/ports/SkOSFile_stdio.cpp (right):

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:38: 
ftell returns a long. Should we use that instead of ssize_t?

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:45: if (size < 0) {
size = 0;  // don't return here, or we won't have restored the previous fseek.

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:48: ::fseek((FILE*)f, (long)curr, SEEK_SET);       
// go back to our prev loc
will probably need to cast in the return...

return static_cast<size_t>(size);

[Steve VanDeBogart, 2012-03-28 00:30:09]

https://codereview.appspot.com/5936047/diff/1/include/core/SkPoint.h
File include/core/SkPoint.h (right):

https://codereview.appspot.com/5936047/diff/1/include/core/SkPoint.h#newcode151
include/core/SkPoint.h:151: bool isZero() const { return (fX == 0) || (fY == 0);
}
On 2012/03/27 20:53:24, bsalomon wrote:
> &&?

Done.

https://codereview.appspot.com/5936047/diff/1/src/gpu/GrAAConvexPathRenderer.cpp
File src/gpu/GrAAConvexPathRenderer.cpp (right):

https://codereview.appspot.com/5936047/diff/1/src/gpu/GrAAConvexPathRenderer....
src/gpu/GrAAConvexPathRenderer.cpp:60: p0.setZero();  // Suppress Coverity
warning.
On 2012/03/27 20:53:24, bsalomon wrote:
> Personally I'd prefer SkPoint p0 = {0, 0}; If we're going to have a zeroing
> function naming it reset() would be consistent with matrix and other types.

SkIPoint already has setZero, that's why I added it to SkPoint - for
consistency. Happy to use what ever kind of initialization you prefer.

https://codereview.appspot.com/5936047/diff/1/src/ports/SkFontHost_FreeType.cpp
File src/ports/SkFontHost_FreeType.cpp (left):

https://codereview.appspot.com/5936047/diff/1/src/ports/SkFontHost_FreeType.c...
src/ports/SkFontHost_FreeType.cpp:1120: getBBoxForCurrentGlyph(glyph, &bbox);
On 2012/03/27 20:54:58, reed1 wrote:
> :( this change seems too bad, as (afaik) FT guarantees to fill out bbox for
us.
> Do we know that there is a code-path where bbox is uninitialized?

This is CID 103631.  Coverity doesn't seem to think that getBBoxForCrrentGlyph
will necessarily fill in the bbox.yMin.  It looked to me like initialization
wouldn't cost much, so I added it.  If you're sure that free type will fill in
all fields in all cases, then the initialization isn't necessary.

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp
File src/ports/SkOSFile_stdio.cpp (right):

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:38: 
On 2012/03/27 20:54:58, reed1 wrote:
> ftell returns a long. Should we use that instead of ssize_t?

Done.

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:45: if (size < 0) {
On 2012/03/27 20:54:58, reed1 wrote:
> size = 0;  // don't return here, or we won't have restored the previous fseek.

Done.

https://codereview.appspot.com/5936047/diff/1/src/ports/SkOSFile_stdio.cpp#ne...
src/ports/SkOSFile_stdio.cpp:48: ::fseek((FILE*)f, (long)curr, SEEK_SET);       
// go back to our prev loc
On 2012/03/27 20:54:58, reed1 wrote:
> will probably need to cast in the return...
> 
> return static_cast<size_t>(size);

Xcode seems to be ok without a cast.

[reed1, 2012-03-28 12:56:23]

https://codereview.appspot.com/5936047/diff/3002/include/core/SkPoint.h
File include/core/SkPoint.h (right):

https://codereview.appspot.com/5936047/diff/3002/include/core/SkPoint.h#newco...
include/core/SkPoint.h:150: */
esp. since float compares are tricky, and we don't seem to be using this, I'd
prefer we don't add the method. However, if we anticipate callers, then lets
perform the compare with the const value on the left.

return (0 == fX) && (0 == fY);

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
File src/gpu/GrAAConvexPathRenderer.cpp (right):

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
src/gpu/GrAAConvexPathRenderer.cpp:55: GrScalar area = 0;
While we're at it, perhaps we init center in the same manner:

SkPoint center = {};

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
src/gpu/GrAAConvexPathRenderer.cpp:59: SkPoint p0;
minor preference for

SkPoint p0 = {};

Brian?

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkFontHost_FreeTyp...
File src/ports/SkFontHost_FreeType.cpp (left):

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkFontHost_FreeTyp...
src/ports/SkFontHost_FreeType.cpp:1119: FT_BBox bbox;
FT_BBox bbox = {};  // suppress Covertly warning

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkOSFile_stdio.cpp
File src/ports/SkOSFile_stdio.cpp (right):

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkOSFile_stdio.cpp...
src/ports/SkOSFile_stdio.cpp:47: }
Don't need the cast on curr now.

[bsalomon, 2012-03-28 13:52:48]

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
File src/gpu/GrAAConvexPathRenderer.cpp (right):

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
src/gpu/GrAAConvexPathRenderer.cpp:55: GrScalar area = 0;
On 2012/03/28 12:56:23, reed1 wrote:
> While we're at it, perhaps we init center in the same manner:
> 
> SkPoint center = {};

My pref would either be "= {};" or "= {0, 0};" for both center and p0.

[Steve VanDeBogart, 2012-03-28 17:00:53]

https://codereview.appspot.com/5936047/diff/3002/include/core/SkPoint.h
File include/core/SkPoint.h (right):

https://codereview.appspot.com/5936047/diff/3002/include/core/SkPoint.h#newco...
include/core/SkPoint.h:150: */
On 2012/03/28 12:56:23, reed1 wrote:
> esp. since float compares are tricky, and we don't seem to be using this, I'd
> prefer we don't add the method. However, if we anticipate callers, then lets
> perform the compare with the const value on the left.
> 
> return (0 == fX) && (0 == fY);

Removed - as you say, a float being zero is tricky proposition.  I only added it
to be symmetrical with SkIRect.

https://codereview.appspot.com/5936047/diff/3002/include/core/SkPoint.h#newco...
include/core/SkPoint.h:156: void setZero() { fX = fY = 0; }
Since I'm no long calling this, I've removed all changes in this file.

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
File src/gpu/GrAAConvexPathRenderer.cpp (right):

https://codereview.appspot.com/5936047/diff/3002/src/gpu/GrAAConvexPathRender...
src/gpu/GrAAConvexPathRenderer.cpp:55: GrScalar area = 0;
On 2012/03/28 13:52:49, bsalomon wrote:
> On 2012/03/28 12:56:23, reed1 wrote:
> > While we're at it, perhaps we init center in the same manner:
> > 
> > SkPoint center = {};
> 
> My pref would either be "= {};" or "= {0, 0};" for both center and p0.
> 

Changed both to "= {};"

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkFontHost_FreeTyp...
File src/ports/SkFontHost_FreeType.cpp (left):

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkFontHost_FreeTyp...
src/ports/SkFontHost_FreeType.cpp:1119: FT_BBox bbox;
On 2012/03/28 12:56:23, reed1 wrote:
> FT_BBox bbox = {};  // suppress Covertly warning

Done.

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkOSFile_stdio.cpp
File src/ports/SkOSFile_stdio.cpp (right):

https://codereview.appspot.com/5936047/diff/3002/src/ports/SkOSFile_stdio.cpp...
src/ports/SkOSFile_stdio.cpp:47: }
On 2012/03/28 12:56:23, reed1 wrote:
> Don't need the cast on curr now.

Done.

[bsalomon, 2012-03-28 17:08:48]

LGTM

[reed1, 2012-03-28 17:12:31]

lgtm