Get rid of NULLs in core/

Get rid of NULLs in core/

Committed: https://pdfium.googlesource.com/pdfium/+/4997b22f84307521a62838f874928bf56cd3423c

[Lei Zhang, 2016-06-02 01:51:38]

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

[Tom Sepez, 2016-06-02 20:09:50]

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

https://codereview.chromium.org/2032613003/diff/1/core/fpdfapi/fpdf_edit/fpdf...
core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:1559: if (m_pEncryptDict && 0 ==
m_pEncryptDict->GetObjNum()) {
nit: mumble == 0 rather than 0 == mumble.

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/codec/fx_codec...
File core/fxcodec/codec/fx_codec_jpeg.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/codec/fx_codec...
core/fxcodec/codec/fx_codec_jpeg.cpp:237: if (pAttribute) {
Agree that pInfo is nonzero in its one caller.

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/lgif/fx_gif.cpp
File core/fxcodec/lgif/fx_gif.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/lgif/fx_gif.cp...
core/fxcodec/lgif/fx_gif.cpp:154: dst_buf = FX_Realloc(uint8_t, dst_buf,
dst_len);
followup: Probably should be a try-realloc here.  Feels like we're willing to
continue with any size buffer here but would prefer the largest we can get.

https://codereview.chromium.org/2032613003/diff/1/core/fxcrt/fx_basic_maps.cpp
File core/fxcrt/fx_basic_maps.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcrt/fx_basic_maps.cp...
core/fxcrt/fx_basic_maps.cpp:80: if (!(pAssoc = GetAssocAt(key, nHash))) {
nit: move assign to previous line.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_form.cpp
File core/fpdfdoc/doc_form.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_form.c...
core/fpdfdoc/doc_form.cpp:226: _Node *pNode = &m_Root, *pLast = nullptr;
nit: one per line while we're at it.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_utils.cpp
File core/fpdfdoc/doc_utils.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_utils....
core/fpdfdoc/doc_utils.cpp:282: CPDF_Font* pFont = nullptr;
nit: maybe just move to line 285.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_viewer...
File core/fpdfdoc/doc_viewerPreferences.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_viewer...
core/fpdfdoc/doc_viewerPreferences.cpp:39: CPDF_Array* pRange = nullptr;
nit: local not needed.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_flate.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_flate.cpp:980: uint8_t* pSrcBuf = nullptr;
nit: combine with 981

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_icc.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_icc.cpp:101: cmsHPROFILE srcProfile = nullptr;
nit:combine with 106.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_jpx_unittest.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_jpx_unittest.cpp:26: // Error codes, not segvs,
should callers pass us a nullptr pointer.
I think NULL is better in comments as it expresses the generic concept of
NULLness rather than nullptr pointer, which is redundant.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
File core/fxcodec/jbig2/JBig2_Image.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
core/fxcodec/jbig2/JBig2_Image.cpp:126: if (nullptr == pSrcRect || *pSrcRect ==
FX_RECT(0, 0, m_nWidth, m_nHeight)) {
nit: !pSrcRect

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
core/fxcodec/jbig2/JBig2_Image.cpp:246: uint8_t *lineSrc = nullptr, *lineDst =
nullptr, *sp = nullptr, *dp = nullptr;
one per line

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/lgif/fx_gi...
File core/fxcodec/lgif/fx_gif.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/lgif/fx_gi...
core/fxcodec/lgif/fx_gif.cpp:1005: if (avial_buf_ptr) {
nit: sp. avial vs avail

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
File core/fxcrt/fx_basic_bstring_unittest.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
core/fxcrt/fx_basic_bstring_unittest.cpp:267: CFX_ByteStringC empty_string(""); 
// Pointer to NUL, not nullptr pointer.
ditto for comment.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
core/fxcrt/fx_basic_bstring_unittest.cpp:761: // Pointer to nul, not nullptr
ptr, hence length 1 ...
ditto

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:73: CFX_CRTFileAccess* pFA = nullptr;
nit combine

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:75: if (nullptr == pFA)
can't happen

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_b...
File core/fxcrt/include/fx_basic.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_b...
core/fxcrt/include/fx_basic.h:479: return (m_nCount == 0) ? nullptr :
(FX_POSITION)-1;
overparenthesized.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
File core/fxcrt/include/fx_memory.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:16: // For external C libraries to malloc through
PDFium. These may return nullptr.
ditto

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:64: // Never returns nullptr.
ditto

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:70: // May return nullptr.
ditto

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_s...
File core/fxcrt/include/fx_system.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_s...
core/fxcrt/include/fx_system.h:94: #ifndef nullptr
Let's not do this.  Let's keep it NULL for things that may include this.

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_appl...
File core/fxge/apple/fx_apple_platform.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_appl...
core/fxge/apple/fx_apple_platform.cpp:130: if (nullptr == ctx) {
nit: !ctx

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
File core/fxge/apple/fx_quartz_device.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:53: if (nullptr == pDataProvider) {
!pDataProvider

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:479: if (nullptr == image) {
!image

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:557: if (nullptr == pBitmap1) {
!pBitmap1

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:593: if (nullptr == pBitmap1) {
ditto

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:675: if (nullptr == pBitmap1) {
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:711: if (nullptr == pBitmap1) {
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:820: if (nullptr == pFont || nullptr ==
m_context)
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:878: if (nullptr == graphState) {
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:993: if (nullptr == m_context) {
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:1033: if (nullptr == m_pContext) {
"

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/win32/fx_win3...
File core/fxge/win32/fx_win32_dib.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/win32/fx_win3...
core/fxge/win32/fx_win32_dib.cpp:105: HBITMAP hBitmap = nullptr;
combine.

[Lei Zhang, 2016-06-07 07:33:24]

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

https://codereview.chromium.org/2032613003/diff/1/core/fpdfapi/fpdf_edit/fpdf...
core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:1559: if (m_pEncryptDict && 0 ==
m_pEncryptDict->GetObjNum()) {
On 2016/06/02 20:09:48, Tom Sepez wrote:
> nit: mumble == 0 rather than 0 == mumble.

Someone else took care of it.

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/codec/fx_codec...
File core/fxcodec/codec/fx_codec_jpeg.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/codec/fx_codec...
core/fxcodec/codec/fx_codec_jpeg.cpp:237: if (pAttribute) {
On 2016/06/02 20:09:48, Tom Sepez wrote:
> Agree that pInfo is nonzero in its one caller.

Acknowledged.

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/lgif/fx_gif.cpp
File core/fxcodec/lgif/fx_gif.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcodec/lgif/fx_gif.cp...
core/fxcodec/lgif/fx_gif.cpp:154: dst_buf = FX_Realloc(uint8_t, dst_buf,
dst_len);
On 2016/06/02 20:09:48, Tom Sepez wrote:
> followup: Probably should be a try-realloc here.  Feels like we're willing to
> continue with any size buffer here but would prefer the largest we can get.

Added TODO

https://codereview.chromium.org/2032613003/diff/1/core/fxcrt/fx_basic_maps.cpp
File core/fxcrt/fx_basic_maps.cpp (right):

https://codereview.chromium.org/2032613003/diff/1/core/fxcrt/fx_basic_maps.cp...
core/fxcrt/fx_basic_maps.cpp:80: if (!(pAssoc = GetAssocAt(key, nHash))) {
On 2016/06/02 20:09:48, Tom Sepez wrote:
> nit: move assign to previous line.

I did in a later CL.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_form.cpp
File core/fpdfdoc/doc_form.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_form.c...
core/fpdfdoc/doc_form.cpp:226: _Node *pNode = &m_Root, *pLast = nullptr;
On 2016/06/02 20:09:48, Tom Sepez wrote:
> nit: one per line while we're at it.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_utils.cpp
File core/fpdfdoc/doc_utils.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_utils....
core/fpdfdoc/doc_utils.cpp:282: CPDF_Font* pFont = nullptr;
On 2016/06/02 20:09:48, Tom Sepez wrote:
> nit: maybe just move to line 285.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_viewer...
File core/fpdfdoc/doc_viewerPreferences.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fpdfdoc/doc_viewer...
core/fpdfdoc/doc_viewerPreferences.cpp:39: CPDF_Array* pRange = nullptr;
On 2016/06/02 20:09:48, Tom Sepez wrote:
> nit: local not needed.  

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_flate.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_flate.cpp:980: uint8_t* pSrcBuf = nullptr;
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit: combine with 981

Code is gone.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_icc.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_icc.cpp:101: cmsHPROFILE srcProfile = nullptr;
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit:combine with 106.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
File core/fxcodec/codec/fx_codec_jpx_unittest.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/codec/fx_c...
core/fxcodec/codec/fx_codec_jpx_unittest.cpp:26: // Error codes, not segvs,
should callers pass us a nullptr pointer.
On 2016/06/02 20:09:49, Tom Sepez wrote:
> I think NULL is better in comments as it expresses the generic concept of
> NULLness rather than nullptr pointer, which is redundant.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
File core/fxcodec/jbig2/JBig2_Image.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
core/fxcodec/jbig2/JBig2_Image.cpp:126: if (nullptr == pSrcRect || *pSrcRect ==
FX_RECT(0, 0, m_nWidth, m_nHeight)) {
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit: !pSrcRect

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/jbig2/JBig...
core/fxcodec/jbig2/JBig2_Image.cpp:246: uint8_t *lineSrc = nullptr, *lineDst =
nullptr, *sp = nullptr, *dp = nullptr;
On 2016/06/02 20:09:49, Tom Sepez wrote:
> one per line

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/lgif/fx_gi...
File core/fxcodec/lgif/fx_gif.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcodec/lgif/fx_gi...
core/fxcodec/lgif/fx_gif.cpp:1005: if (avial_buf_ptr) {
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit: sp. avial vs avail

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
File core/fxcrt/fx_basic_bstring_unittest.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
core/fxcrt/fx_basic_bstring_unittest.cpp:267: CFX_ByteStringC empty_string(""); 
// Pointer to NUL, not nullptr pointer.
On 2016/06/02 20:09:49, Tom Sepez wrote:
> ditto for comment.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_basic_bst...
core/fxcrt/fx_basic_bstring_unittest.cpp:761: // Pointer to nul, not nullptr
ptr, hence length 1 ...
On 2016/06/02 20:09:49, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:73: CFX_CRTFileAccess* pFA = nullptr;
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit combine

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:75: if (nullptr == pFA)
On 2016/06/02 20:09:49, Tom Sepez wrote:
> can't happen

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_b...
File core/fxcrt/include/fx_basic.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_b...
core/fxcrt/include/fx_basic.h:479: return (m_nCount == 0) ? nullptr :
(FX_POSITION)-1;
On 2016/06/02 20:09:49, Tom Sepez wrote:
> overparenthesized.

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
File core/fxcrt/include/fx_memory.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:16: // For external C libraries to malloc through
PDFium. These may return nullptr.
On 2016/06/02 20:09:49, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:64: // Never returns nullptr.
On 2016/06/02 20:09:49, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_m...
core/fxcrt/include/fx_memory.h:70: // May return nullptr.
On 2016/06/02 20:09:49, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_s...
File core/fxcrt/include/fx_system.h (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxcrt/include/fx_s...
core/fxcrt/include/fx_system.h:94: #ifndef nullptr
On 2016/06/02 20:09:49, Tom Sepez wrote:
> Let's not do this.  Let's keep it NULL for things that may include this.

I ended up removing this in one of the later patch sets and all the trybots were
green.

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_appl...
File core/fxge/apple/fx_apple_platform.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_appl...
core/fxge/apple/fx_apple_platform.cpp:130: if (nullptr == ctx) {
On 2016/06/02 20:09:49, Tom Sepez wrote:
> nit: !ctx

Done.

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
File core/fxge/apple/fx_quartz_device.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/apple/fx_quar...
core/fxge/apple/fx_quartz_device.cpp:53: if (nullptr == pDataProvider) {
On 2016/06/02 20:09:50, Tom Sepez wrote:
> !pDataProvider

Done. Not sure how I missed these.

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/win32/fx_win3...
File core/fxge/win32/fx_win32_dib.cpp (right):

https://codereview.chromium.org/2032613003/diff/20001/core/fxge/win32/fx_win3...
core/fxge/win32/fx_win32_dib.cpp:105: HBITMAP hBitmap = nullptr;
On 2016/06/02 20:09:50, Tom Sepez wrote:
> combine.

Done.

[Tom Sepez, 2016-06-07 16:06:33]

lgtm

[Lei Zhang, 2016-06-07 17:45:39]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-06-07 17:45:49]

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

[commit-bot: I haz the power, 2016-06-07 17:46:26]

Description was changed from

==========
Get rid of NULLs in core/
==========

to

==========
Get rid of NULLs in core/

Committed:
https://pdfium.googlesource.com/pdfium/+/4997b22f84307521a62838f874928bf56cd3...
==========

[commit-bot: I haz the power, 2016-06-07 17:46:27]

Committed patchset #9 (id:150001) as
https://pdfium.googlesource.com/pdfium/+/4997b22f84307521a62838f874928bf56cd3...