Re-enable MSVC warning 4702

Re-enable MSVC warning 4702

BUG=pdfium:29
R=tsepez@chromium.org

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

[Wei Li, 2016-03-22 22:24:54]

Patchset #1 (id:1) has been deleted

[Wei Li, 2016-03-22 22:27:42]

Patchset #1 (id:20001) has been deleted

[Wei Li, 2016-03-22 22:35:32]

Patchset #1 (id:40001) has been deleted

[Wei Li, 2016-03-22 22:36:10]

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

[Wei Li, 2016-03-22 22:36:10]

Original code in JBig2_GsidProc.cpp should be a bug. PTAL.

[Tom Sepez, 2016-03-22 23:17:35]

lgtm

https://codereview.chromium.org/1821423002/diff/60001/core/fxcodec/jbig2/JBig...
File core/fxcodec/jbig2/JBig2_GsidProc.cpp (right):

https://codereview.chromium.org/1821423002/diff/60001/core/fxcodec/jbig2/JBig...
core/fxcodec/jbig2/JBig2_GsidProc.cpp:101: return nullptr;
nice.

https://codereview.chromium.org/1821423002/diff/60001/third_party/base/logging.h
File third_party/base/logging.h (right):

https://codereview.chromium.org/1821423002/diff/60001/third_party/base/loggin...
third_party/base/logging.h:12: #define CHECK(condition) \
nit: maybe in this file, we do
#ifndef _WIN32
#define NULL_DEREF_IF_POSSIBLE() \
  *(reinterpret_cast<volatile char*>(NULL) + 42) = 0x42;
#else
#define NULL_DEREF_IF_POSSIBLE()
#endif

Then all the other macros and uses in the code don't require ifdefs, they just
do NULL_DEREF_IF_POSSIBLE()

[Tom Sepez, 2016-03-22 23:29:49]

On 2016/03/22 23:17:35, Tom Sepez wrote:
> lgtm
> 
>
https://codereview.chromium.org/1821423002/diff/60001/core/fxcodec/jbig2/JBig...
> File core/fxcodec/jbig2/JBig2_GsidProc.cpp (right):
> 
>
https://codereview.chromium.org/1821423002/diff/60001/core/fxcodec/jbig2/JBig...
> core/fxcodec/jbig2/JBig2_GsidProc.cpp:101: return nullptr;
> nice.
> 
>
https://codereview.chromium.org/1821423002/diff/60001/third_party/base/logging.h
> File third_party/base/logging.h (right):
> 
>
https://codereview.chromium.org/1821423002/diff/60001/third_party/base/loggin...
> third_party/base/logging.h:12: #define CHECK(condition) \
> nit: maybe in this file, we do
> #ifndef _WIN32
> #define NULL_DEREF_IF_POSSIBLE() \
>   *(reinterpret_cast<volatile char*>(NULL) + 42) = 0x42;
> #else
> #define NULL_DEREF_IF_POSSIBLE()
> #endif
> 
> Then all the other macros and uses in the code don't require ifdefs, they just
> do NULL_DEREF_IF_POSSIBLE()

Or NULL_DEREF_IF_POSSIBLE_AFTER_ABORT()  if we want to put all the documentation
into the name ...

[Wei Li, 2016-03-23 00:34:47]

Patchset #2 (id:80001) has been deleted

[Wei Li, 2016-03-23 00:35:56]

thanks

https://codereview.chromium.org/1821423002/diff/60001/third_party/base/logging.h
File third_party/base/logging.h (right):

https://codereview.chromium.org/1821423002/diff/60001/third_party/base/loggin...
third_party/base/logging.h:12: #define CHECK(condition) \
On 2016/03/22 23:17:34, Tom Sepez wrote:
> nit: maybe in this file, we do
> #ifndef _WIN32
> #define NULL_DEREF_IF_POSSIBLE() \
>   *(reinterpret_cast<volatile char*>(NULL) + 42) = 0x42;
> #else
> #define NULL_DEREF_IF_POSSIBLE()
> #endif
> 
> Then all the other macros and uses in the code don't require ifdefs, they just
> do NULL_DEREF_IF_POSSIBLE()

Done.

[Wei Li, 2016-03-23 02:19:12]

Description was changed from

==========
Re-enable MSVC warning 4702

BUG=pdfium:29
==========

to

==========
Re-enable MSVC warning 4702

BUG=pdfium:29
R=tsepez@chromium.org

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

[Wei Li, 2016-03-23 02:19:13]

Committed patchset #2 (id:100001) manually as
e91afbabec3b7c891b464efd221748edaba3c2bc (presubmit successful).

[Tom Sepez, 2016-03-23 16:49:50]

https://codereview.chromium.org/1821423002/diff/100001/core/fxcrt/fx_basic_me...
File core/fxcrt/fx_basic_memmgr.cpp (right):

https://codereview.chromium.org/1821423002/diff/100001/core/fxcrt/fx_basic_me...
core/fxcrt/fx_basic_memmgr.cpp:25: reinterpret_cast<void (*)()>(0xbd)();
This could be NULL_DEREF_IF_POSSIBLE, too.

[Wei Li, 2016-03-23 18:15:12]

https://codereview.chromium.org/1821423002/diff/100001/core/fxcrt/fx_basic_me...
File core/fxcrt/fx_basic_memmgr.cpp (right):

https://codereview.chromium.org/1821423002/diff/100001/core/fxcrt/fx_basic_me...
core/fxcrt/fx_basic_memmgr.cpp:25: reinterpret_cast<void (*)()>(0xbd)();
On 2016/03/23 16:49:50, Tom Sepez wrote:
> This could be NULL_DEREF_IF_POSSIBLE, too.

I thought this was intentional to be set as a different address to be easily
identifiable. Not sure how important it is though.