Fix all the code which has duplicate variable declarations

Fix all the code which has duplicate variable declarations

When there are duplicate variable declarations, the inner names shadow the
outter ones. This is error prone and harder to read. Remove all the
instances found by /analyze.

BUG=chromium:613623, chromium:427616
Committed: https://pdfium.googlesource.com/pdfium/+/db444d2063df6c574882d9263e885c4fe1134133

[Wei Li, 2016-06-01 20:35:51]

Patchset #1 (id:1) has been deleted

[Wei Li, 2016-06-01 21:33:02]

Patchset #3 (id:60001) has been deleted

[Wei Li, 2016-06-01 21:46:10]

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

[Wei Li, 2016-06-01 21:46:11]

pls review, thanks

[Tom Sepez, 2016-06-01 22:23:04]

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_cidfont.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_cidfont.cpp:818: if (CPDF_Array* array_elem =
pObj->AsArray()) {
nit: pSomething, also array_elem feels like a scalar, maybe pInnerArray?

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1142: if (!m_pResources) {
nit: no {}

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1147: if (!pDict)
nit: ? operator might be cleaner.

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1152: if (CPDF_Dictionary* pDict =
m_pResources->GetDictBy(type))
either way, we're going to get m_pResources->GetDictBy(type), so call the pDict
an put it at line 1145.

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1157: CPDF_Dictionary* pDict =
m_pPageResources->GetDictBy(type);
Then we can call this pPageDict and avoid collisions.

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

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp:435: for (int j = 0; j <
key_len; j++)
probably a signed / unsigned comparison here, I'd make these int32_t's, leave
the existing loop logic, and static_cast to uint8_t before the xor.

Also below.

[Tom Sepez, 2016-06-01 22:49:21]

https://codereview.chromium.org/2027273002/diff/80001/core/fxge/dib/fx_dib_co...
File core/fxge/dib/fx_dib_composite.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fxge/dib/fx_dib_co...
core/fxge/dib/fx_dib_composite.cpp:3591: uint8_t dest_scan_o[3] = {dest_scan[2],
dest_scan[1], dest_scan[0]};
nit: can we be consistent about initializer syntax ?
maybe uint8_t scan[3] = { src_b, src_g, src_r }; throughout?

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
File fpdfsdk/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
fpdfsdk/fpdf_transformpage.cpp:150: pArray = pDirectObj->AsArray();
nit: can we use a new variable name here for clarity?

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
fpdfsdk/fpdf_transformpage.cpp:320: pArray = pDirectObj->AsArray();
same here.

[Wei Li, 2016-06-01 23:40:13]

thanks!

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_cidfont.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_cidfont.cpp:818: if (CPDF_Array* array_elem =
pObj->AsArray()) {
On 2016/06/01 22:23:04, Tom Sepez wrote:
> nit: pSomething, also array_elem feels like a scalar, maybe pInnerArray?

Done.

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1142: if (!m_pResources) {
On 2016/06/01 22:23:04, Tom Sepez wrote:
> nit: no {}

Done.

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1147: if (!pDict)
On 2016/06/01 22:23:04, Tom Sepez wrote:
> nit: ? operator might be cleaner.

See below

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1152: if (CPDF_Dictionary* pDict =
m_pResources->GetDictBy(type))
On 2016/06/01 22:23:04, Tom Sepez wrote:
> either way, we're going to get m_pResources->GetDictBy(type), so call the
pDict
> an put it at line 1145.

Thanks for suggesting this, now it looks much clearer.

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1157: CPDF_Dictionary* pDict =
m_pPageResources->GetDictBy(type);
On 2016/06/01 22:23:04, Tom Sepez wrote:
> Then we can call this pPageDict and avoid collisions.

Done.

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

https://codereview.chromium.org/2027273002/diff/80001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp:435: for (int j = 0; j <
key_len; j++)
On 2016/06/01 22:23:04, Tom Sepez wrote:
> probably a signed / unsigned comparison here, I'd make these int32_t's, leave
> the existing loop logic, and static_cast to uint8_t before the xor.
> 
> Also below.

Done.

https://codereview.chromium.org/2027273002/diff/80001/core/fxge/dib/fx_dib_co...
File core/fxge/dib/fx_dib_composite.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/core/fxge/dib/fx_dib_co...
core/fxge/dib/fx_dib_composite.cpp:3591: uint8_t dest_scan_o[3] = {dest_scan[2],
dest_scan[1], dest_scan[0]};
On 2016/06/01 22:49:21, Tom Sepez wrote:
> nit: can we be consistent about initializer syntax ?
> maybe uint8_t scan[3] = { src_b, src_g, src_r }; throughout?

Sure, since src_{b,g,r} are of int type, it may not look very pretty.

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
File fpdfsdk/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
fpdfsdk/fpdf_transformpage.cpp:150: pArray = pDirectObj->AsArray();
On 2016/06/01 22:49:21, Tom Sepez wrote:
> nit: can we use a new variable name here for clarity?

Done.

https://codereview.chromium.org/2027273002/diff/80001/fpdfsdk/fpdf_transformp...
fpdfsdk/fpdf_transformpage.cpp:320: pArray = pDirectObj->AsArray();
On 2016/06/01 22:49:21, Tom Sepez wrote:
> same here.

Done.

[Tom Sepez, 2016-06-02 20:32:20]

lgtm

[Wei Li, 2016-06-02 22:38:50]

The CQ bit was checked by weili@chromium.org

[Wei Li, 2016-06-02 22:38:50]

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

[commit-bot: I haz the power, 2016-06-02 22:38:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2027273002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2027273002/120001

[commit-bot: I haz the power, 2016-06-02 22:48:24]

Description was changed from

==========
Fix all the code which has duplicate variable declarations

When there are duplicate variable declarations, the inner names shadow the
outter ones. This is error prone and harder to read. Remove all the
instances found by /analyze.

BUG=chromium:613623, chromium:427616
==========

to

==========
Fix all the code which has duplicate variable declarations

When there are duplicate variable declarations, the inner names shadow the
outter ones. This is error prone and harder to read. Remove all the
instances found by /analyze.

BUG=chromium:613623, chromium:427616

Committed:
https://pdfium.googlesource.com/pdfium/+/db444d2063df6c574882d9263e885c4fe113...
==========

[commit-bot: I haz the power, 2016-06-02 22:48:25]

Committed patchset #5 (id:120001) as
https://pdfium.googlesource.com/pdfium/+/db444d2063df6c574882d9263e885c4fe113...