Fix more bugs found by /analyze tool

Fix more bugs found by /analyze tool

Three more bugs are discovered:
-- potientially used freed pointer;
-- potientially used uninitialized variable;
-- Used '&&' instead of bitwise operator '&'

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

[Wei Li, 2016-06-03 18:25:12]

Description was changed from

==========
Fix more bugs found by /analyze tool

Three more bugs are discovered:
-- potientially use freed pointer;
-- potientially use uninitialized variable;
-- Use '&&' instead of bitwise operator '&'

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

to

==========
Fix more bugs found by /analyze tool

Three more bugs are discovered:
-- potientially used freed pointer;
-- potientially used uninitialized variable;
-- Used '&&' instead of bitwise operator '&'

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

[Wei Li, 2016-06-03 18:40:13]

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

[Wei Li, 2016-06-03 18:40:14]

pls review, thanks

[Tom Sepez, 2016-06-03 18:54:37]

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp (left):

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:63: FX_Free(pDecryptedData);
Why is OK to remove this?  We might leak.

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp (right):

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:74: if (!m_pStream)
nit: maybe ? oper here instead.

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:82: if (!m_pStream)
ditto

https://codereview.chromium.org/2040503002/diff/20001/xfa/fwl/core/cfwl_widge...
File xfa/fwl/core/cfwl_widgetmgr.cpp (right):

https://codereview.chromium.org/2040503002/diff/20001/xfa/fwl/core/cfwl_widge...
xfa/fwl/core/cfwl_widgetmgr.cpp:772: sizeof(FWL_NEEDREPAINTHITDATA) *
kNeedRepaintHitPoints);
just sizeof(hitPoint)

https://codereview.chromium.org/2040503002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2040503002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
xfa/fxfa/app/xfa_ffchoicelist.cpp:94: if (!(pListBox->GetItemStates(hlistItem) &
FWL_ITEMSTATE_LTB_Selected))
Note: this was probably a real bug.

[Wei Li, 2016-06-03 19:35:02]

Patchset #3 (id:40001) has been deleted

[Wei Li, 2016-06-03 19:48:21]

ptal, thanks

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp (left):

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:63: FX_Free(pDecryptedData);
On 2016/06/03 18:54:37, Tom Sepez wrote:
> Why is OK to remove this?  We might leak.

pDecryptedData is always same as pre-freed pSrcData. Now removed pDecryptedData
to make it clearer.

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp (right):

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:74: if (!m_pStream)
On 2016/06/03 18:54:37, Tom Sepez wrote:
> nit: maybe ? oper here instead.

Done.

https://codereview.chromium.org/2040503002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_stream_acc.cpp:82: if (!m_pStream)
On 2016/06/03 18:54:37, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/2040503002/diff/20001/xfa/fwl/core/cfwl_widge...
File xfa/fwl/core/cfwl_widgetmgr.cpp (right):

https://codereview.chromium.org/2040503002/diff/20001/xfa/fwl/core/cfwl_widge...
xfa/fwl/core/cfwl_widgetmgr.cpp:772: sizeof(FWL_NEEDREPAINTHITDATA) *
kNeedRepaintHitPoints);
On 2016/06/03 18:54:37, Tom Sepez wrote:
> just sizeof(hitPoint)

Done.

This one is a real bug too, hitPoint's content such as bNotContainByDirty was
used without be initialized. The
state is unknown.

[Tom Sepez, 2016-06-03 20:37:35]

lgtm

[Wei Li, 2016-06-03 20:54:04]

The CQ bit was checked by weili@chromium.org

[commit-bot: I haz the power, 2016-06-03 20:54:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2040503002/60001

[commit-bot: I haz the power, 2016-06-03 21:06:39]

Description was changed from

==========
Fix more bugs found by /analyze tool

Three more bugs are discovered:
-- potientially used freed pointer;
-- potientially used uninitialized variable;
-- Used '&&' instead of bitwise operator '&'

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

to

==========
Fix more bugs found by /analyze tool

Three more bugs are discovered:
-- potientially used freed pointer;
-- potientially used uninitialized variable;
-- Used '&&' instead of bitwise operator '&'

BUG=chromium:613623, chromium:427616

Committed:
https://pdfium.googlesource.com/pdfium/+/8d4e0d47f4cd0325be84ebf310fd8de98976...
==========

[commit-bot: I haz the power, 2016-06-03 21:06:40]

Committed patchset #3 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/8d4e0d47f4cd0325be84ebf310fd8de98976...