[PDF] Fix font metric array initialization.

[PDF] Fix font metric array initialization.

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

[epoger, 2012-03-23 12:59:56]

LGTM

[reed1, 2012-03-23 13:35:10]

optionally, could call sk_bzero(ptr, size). Slightly more clear, and matches a
semi-common skia pattern.

[Steve VanDeBogart, 2012-03-23 15:17:22]

On 2012/03/23 13:35:10, reed1 wrote:
> optionally, could call sk_bzero(ptr, size). Slightly more clear, and matches a
> semi-common skia pattern.

SG - Done.

[thakis, 2012-03-26 23:35:23]

https://codereview.appspot.com/5885055/diff/5001/src/pdf/SkPDFDocument.cpp
File src/pdf/SkPDFDocument.cpp (right):

https://codereview.appspot.com/5885055/diff/5001/src/pdf/SkPDFDocument.cpp#ne...
src/pdf/SkPDFDocument.cpp:228: sk_bzero(counts, sizeof(counts));
This is actually not correct. The C spec says that sizeof an array that's passed
as a function parameter is just the size of a pointer (see e.g.
http://stackoverflow.com/questions/1328223/sizeof-array-passed-as-parameter ),
so this will only zero out the first 4 bytes.

Since this is so surprising, I added a warning to clang that calls this out:


../../third_party/skia/src/pdf/SkPDFDocument.cpp:228:28: warning: sizeof on
array function parameter will return size of 'int *' instead of 'int [6]'
[-Wsizeof-array-argument]
    sk_bzero(counts, sizeof(counts));
                           ^
../../third_party/skia/src/pdf/SkPDFDocument.cpp:227:13: note: declared here
        int counts[SkAdvancedTypefaceMetrics::kNotEmbeddable_Font + 1]) const {
            ^
1 warning generated.


Can you take a look?

[Steve VanDeBogart, 2012-03-26 23:41:27]

https://codereview.appspot.com/5885055/diff/5001/src/pdf/SkPDFDocument.cpp
File src/pdf/SkPDFDocument.cpp (right):

https://codereview.appspot.com/5885055/diff/5001/src/pdf/SkPDFDocument.cpp#ne...
src/pdf/SkPDFDocument.cpp:228: sk_bzero(counts, sizeof(counts));
On 2012/03/26 23:35:24, thakis wrote:
> This is actually not correct. The C spec says that sizeof an array that's
passed
> as a function parameter is just the size of a pointer (see e.g.
> http://stackoverflow.com/questions/1328223/sizeof-array-passed-as-parameter ),
> so this will only zero out the first 4 bytes.
> 
> Since this is so surprising, I added a warning to clang that calls this out:
> 
> 
> ../../third_party/skia/src/pdf/SkPDFDocument.cpp:228:28: warning: sizeof on
> array function parameter will return size of 'int *' instead of 'int [6]'
> [-Wsizeof-array-argument]
>     sk_bzero(counts, sizeof(counts));
>                            ^
> ../../third_party/skia/src/pdf/SkPDFDocument.cpp:227:13: note: declared here
>         int counts[SkAdvancedTypefaceMetrics::kNotEmbeddable_Font + 1]) const
{
>             ^
> 1 warning generated.
> 
> 
> Can you take a look?

In fact I worked around the initial bug on the Chrome side to make it easier to
roll Skia.  Skia has now rolled passed this revision so I dispatched a change to
clean up the Chrome side which failed on the bots.  I suspected that the problem
might be what you describ and I was about to attach my debugger to confirm that.

I'll dispatch a change to go back to a (correct version) of what it was before.