Cleanup: Do not check pointers before deleting them.

Cleanup: Do not check pointers before deleting them.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/cfac954abcab7caf47d3fa3d641c553cba998271

[Lei Zhang, 2015-06-19 20:08:32]

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

[Tom Sepez, 2015-06-19 21:13:37]

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:2123: if ((m_bStandardSecurity
|| m_bNewCrypto)) {
nit: double ((

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:2126: m_pCryptoHandler = NULL;
I don't think you can pull this out of the if.  If we had a handler, and neither
m_bWhatever was set, we'd still have a handler here in the old code.

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:315: m_pSecurityHandler =
NULL;
still need to check !m_bForceUseSecurityHandler

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:1481: delete
m_pSecurityHandler;
same here.

[Tom Sepez, 2015-06-19 21:15:29]

Thanks for doing this.  I took a stab at it a while ago, but I got hung up on
making as many of these unique_ptrs as possible.  Probably better in two steps

[Lei Zhang, 2015-06-19 21:16:35]

On 2015/06/19 21:15:29, Tom Sepez wrote:
> Thanks for doing this.  I took a stab at it a while ago, but I got hung up on
> making as many of these unique_ptrs as possible.  Probably better in two steps

There's also places where I'd like to return an unique_ptr, but that's not
currently possible with our simple impl.

[Lei Zhang, 2015-06-19 21:30:37]

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:2123: if ((m_bStandardSecurity
|| m_bNewCrypto)) {
On 2015/06/19 21:13:37, Tom Sepez wrote:
> nit: double ((

Done.

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:2126: m_pCryptoHandler = NULL;
On 2015/06/19 21:13:37, Tom Sepez wrote:
> I don't think you can pull this out of the if.  If we had a handler, and
neither
> m_bWhatever was set, we'd still have a handler here in the old code.

Done.

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:315: m_pSecurityHandler =
NULL;
On 2015/06/19 21:13:37, Tom Sepez wrote:
> still need to check !m_bForceUseSecurityHandler

Done.

https://codereview.chromium.org/1192743004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:1481: delete
m_pSecurityHandler;
On 2015/06/19 21:13:37, Tom Sepez wrote:
> same here.

Done. I'm going to look the other way and prevent line 1480 isn't there for now.

[Tom Sepez, 2015-06-19 21:38:35]

lgtm

[Lei Zhang, 2015-06-19 21:51:15]

Committed patchset #4 (id:60001) manually as
cfac954abcab7caf47d3fa3d641c553cba998271 (presubmit successful).