Fix uninitialized coords and one of infinite loops

Fix uninitialized coords and one of infinite loops

BUG=387854
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/10ec7ca

[Tom Sepez, 2014-07-14 21:38:01]

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:667: FX_BOOL
_CheckCoonTensorPara(CPDF_MeshStream &stream)
nit: static

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:669: FX_BOOL bCoorBits = (
stream.m_nCoordBits== 1  ||
nit: space before ==

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:707: if
(!_CheckCoonTensorPara(stream))
nit: { } per other tests that just return in this function.

[Tom Sepez, 2014-07-15 18:27:26]

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:667: FX_BOOL
_CheckCoonTensorPara(CPDF_MeshStream &stream)
nit: const CPDF_MeshStream&

[Tom Sepez, 2014-07-15 18:35:45]

LGTM with a few nits.

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:722: {
nit: brace on previous line to match style.

https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:735: for (int i = 0; i < 4;
i ++) {
nit: this declaration of i shadows an outer one at line 730, and can be removed,
using the outer one.

[jun_fang, 2014-07-15 22:12:24]

On 2014/07/15 18:35:45, Tom Sepez wrote:
> LGTM with a few nits.
> 
>
https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
> File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):
> 
>
https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
> core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:722: {
> nit: brace on previous line to match style.
> 
>
https://codereview.chromium.org/372453005/diff/1/core/src/fpdfapi/fpdf_render...
> core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:735: for (int i = 0; i <
4;
> i ++) {
> nit: this declaration of i shadows an outer one at line 730, and can be
removed,
> using the outer one.

Thanks for your review. This issue includes uninitialized coords and several
infinite loops (at least two). I fixed one of them. Shall I deliver this partial
fix first or wait for the final fix?

[Tom Sepez, 2014-07-15 22:14:05]

> Shall I deliver this partial fix first or wait for the final fix?
Its fine to land this in pieces, if you'd like.

[jun_fang, 2014-07-15 23:55:51]

On 2014/07/15 22:14:05, Tom Sepez wrote:
> > Shall I deliver this partial fix first or wait for the final fix?
> Its fine to land this in pieces, if you'd like.

I updated the change based on your review comments.

[Tom Sepez, 2014-07-22 17:27:21]

Still LGTM, but a few new nits.

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:708: if
(!_CheckCoonTensorPara(stream)){
nit: space before { to match existing style in file.

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:722: for (int i = 0; i <
16; i ++){
nit: space before { to match existing style in file.

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:729: int iStartPoint = 0,
iStartColor = 0, i = 0;
nit: initialized value isn't used.  reset to 0 in for loop.

[Tom Sepez, 2014-07-22 17:28:52]

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp (right):

https://codereview.chromium.org/372453005/diff/20001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:669: static FX_BOOL
_CheckCoonTensorPara(CPDF_MeshStream& stream)
nit: const CPDF_MeshStream&

[jun_fang, 2014-07-30 18:42:29]

Hi Tom, please review my update. Thanks!

[Tom Sepez, 2014-07-30 18:51:51]

LGTM++. Thanks for removing some more of those unused locals.

[jun_fang, 2014-07-31 06:13:19]

Committed patchset #3 manually as r10ec7ca (presubmit successful).