[OTS] cmap ParseFormat4() should check the first segment

cmap ParseFormat4() should check the first segment

R=yusukes@chromium.org,agl@chromium.org
BUG=chromium:146254
TEST=ran test/test_{un,}malicious_fonts.sh

[bashi, 2012-09-04 01:16:16]

Hi Yusuke-san, agl,

Could you take a look? I read
http://www.microsoft.com/typography/otspec/cmap.htm and I think we should check
glyph indexes of the first segment of format 4 subtable. I checked the CL with
valid 4000+ fonts. The CL doesn't reject all valid fonts.

[Yusuke Sato, 2012-09-04 04:33:08]

lgtm

On 2012/09/04 01:16:16, bashik wrote:
> Hi Yusuke-san, agl,
> 
> Could you take a look? I read
> http://www.microsoft.com/typography/otspec/cmap.htm and I think we should
check
> glyph indexes of the first segment of format 4 subtable. I checked the CL with
> valid 4000+ fonts. The CL doesn't reject all valid fonts.

[agl, 2012-09-04 13:30:05]

LGTM

http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc
File src/cmap.cc (right):

http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc#newcode215
src/cmap.cc:215: for (unsigned i = 0; i < segcount; ++i) {
Was there a reason why we didn't check the 0-index table previously?

[Yusuke Sato, 2012-09-04 17:43:05]

http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc
File src/cmap.cc (right):

http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc#newcode215
src/cmap.cc:215: for (unsigned i = 0; i < segcount; ++i) {
On 2012/09/04 13:30:05, agl wrote:
> Was there a reason why we didn't check the 0-index table previously?

I guess this is just a careless mistake...  Line 172 above uses 'for (unsigned i
= 1; i < segcount; ++i)' for a valid reason, and we (I??) somehow
copy-and-pasted it?

[bashi, 2012-09-05 10:52:01]

On 2012/09/04 17:43:05, Yusuke Sato wrote:
> http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc
> File src/cmap.cc (right):
> 
> http://codereview.chromium.org/10913058/diff/3001/src/cmap.cc#newcode215
> src/cmap.cc:215: for (unsigned i = 0; i < segcount; ++i) {
> On 2012/09/04 13:30:05, agl wrote:
> > Was there a reason why we didn't check the 0-index table previously?
> 
> I guess this is just a careless mistake...  Line 172 above uses 'for (unsigned
i
> = 1; i < segcount; ++i)' for a valid reason, and we (I??) somehow
> copy-and-pasted it?

Agreed. I think it's a mistake. svn blame says that the line was a part of
"Initial commit".

[agl, 2012-09-05 14:07:23]

On Wed, Sep 5, 2012 at 6:52 AM,  <bashi@chromium.org> wrote:
> Agreed. I think it's a mistake. svn blame says that the line was a part of
> "Initial commit".

Opps :(


Cheers

AGL