Clean up fx_codec_fax.cpp.

Clean up fx_codec_fax.cpp.

Committed: https://pdfium.googlesource.com/pdfium/+/2ab466e3a59d845422e06e50bc38d4bf31ac1adf

[Lei Zhang, 2016-09-23 05:15:21]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 05:15:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 05:17:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 05:17:53]

Dry run: Try jobs failed on following builders:
  mac_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa/builds/...)

[Lei Zhang, 2016-09-23 06:50:55]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 06:50:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 08:51:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 08:51:32]

Dry run: Try jobs failed on following builders:
  mac on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  mac_no_v8 on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  mac_xfa on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  win on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  win_no_v8 on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  win_xfa on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)
  win_xfa_clang on master.tryserver.client.pdfium (JOB_TIMED_OUT, no build URL)

[Lei Zhang, 2016-09-23 20:50:52]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 20:50:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-09-23 21:08:46]

thestig@chromium.org changed reviewers:
+ kcwu@chromium.org, tsepez@chromium.org

[commit-bot: I haz the power, 2016-09-23 21:12:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 21:12:07]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-09-23 21:55:48]

lgtm

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp (right):

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:185: if (dest_size < old)
note: OK, unsigned type.

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:187: i += src_buf[i] + 2;
note: there's one value where this can overflow, even if dest_size didn't. let's
just ignore it?

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:244: EndOfLine =
!!pParams->GetIntegerFor("EndOfLine");
Yay, another not assigning 0 or 1 to FX_BOOL !!!

https://codereview.chromium.org/2357173005/diff/80001/testing/libfuzzer/pdf_c...
File testing/libfuzzer/pdf_codec_fax_fuzzer.cc (right):

https://codereview.chromium.org/2357173005/diff/80001/testing/libfuzzer/pdf_c...
testing/libfuzzer/pdf_codec_fax_fuzzer.cc:25: bool EndOfLine = (data[20] & 0x01)
== 0;
nit: prefer !

[kcwu, 2016-09-24 03:00:57]

lgtm

https://codereview.chromium.org/2357173005/diff/80001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_fax.cpp (right):

https://codereview.chromium.org/2357173005/diff/80001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_fax.cpp:608: // Should not overflow. Checked by
CCodec_FaxDecoder::CreateDecoder.
Delete this comment.
My mistake, what I meant to say is CCodec_FaxModule::CreateDecoder (here).

[Lei Zhang, 2016-09-26 22:50:18]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 22:50:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-09-26 22:50:59]

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp (right):

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:187: i += src_buf[i] + 2;
On 2016/09/23 21:55:48, Tom Sepez wrote:
> note: there's one value where this can overflow, even if dest_size didn't.
let's
> just ignore it?

Sure.

https://codereview.chromium.org/2357173005/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp:244: EndOfLine =
!!pParams->GetIntegerFor("EndOfLine");
On 2016/09/23 21:55:48, Tom Sepez wrote:
> Yay, another not assigning 0 or 1 to FX_BOOL !!!

Acknowledged.

https://codereview.chromium.org/2357173005/diff/80001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_fax.cpp (right):

https://codereview.chromium.org/2357173005/diff/80001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_fax.cpp:608: // Should not overflow. Checked by
CCodec_FaxDecoder::CreateDecoder.
On 2016/09/24 03:00:57, kcwu wrote:
> Delete this comment.
> My mistake, what I meant to say is CCodec_FaxModule::CreateDecoder (here).

Done.

https://codereview.chromium.org/2357173005/diff/80001/testing/libfuzzer/pdf_c...
File testing/libfuzzer/pdf_codec_fax_fuzzer.cc (right):

https://codereview.chromium.org/2357173005/diff/80001/testing/libfuzzer/pdf_c...
testing/libfuzzer/pdf_codec_fax_fuzzer.cc:25: bool EndOfLine = (data[20] & 0x01)
== 0;
On 2016/09/23 21:55:48, Tom Sepez wrote:
> nit: prefer !

Didn't you review this the first time around? :-P

[commit-bot: I haz the power, 2016-09-26 22:52:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 22:52:34]

Dry run: Try jobs failed on following builders:
  android on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/android/builds/898)

[Lei Zhang, 2016-09-26 22:53:33]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 22:53:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 23:11:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 23:11:27]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-09-26 23:14:10]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-09-26 23:14:11]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
kcwu@chromium.org
Link to the patchset: https://codereview.chromium.org/2357173005/#ps120001
(title: "typo")

[commit-bot: I haz the power, 2016-09-26 23:14:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 23:14:33]

Description was changed from

==========
Clean up fx_codec_fax.cpp.
==========

to

==========
Clean up fx_codec_fax.cpp.

Committed:
https://pdfium.googlesource.com/pdfium/+/2ab466e3a59d845422e06e50bc38d4bf31ac...
==========

[commit-bot: I haz the power, 2016-09-26 23:14:34]

Committed patchset #7 (id:120001) as
https://pdfium.googlesource.com/pdfium/+/2ab466e3a59d845422e06e50bc38d4bf31ac...