Don't leave dangling pointer to out-of-scope local in CPDF_StreamContentParser::Parse.

Don't leave dangling pointer to out-of-scope local in CPDF_StreamContentParser::Parse.

This is just a bit of defensive programming; I'm not sure the situation can
occur in the current code, but the following code is likely to set off a red
flag to anyone who reads it:
    CPDF_StreamParser syntax(pData, dwSize);
    m_pSyntax = &syntax;
since the extent of the local |syntax| is far less than the pointer member
|m_pSyntax|.  NULL it out before syntax goes out of scope.

R=jun_fang@foxitsoftware.com

Committed: https://pdfium.googlesource.com/pdfium/+/35d720aff01c5ea778c16ac1e31c56f68490f10b

[Tom Sepez, 2014-10-13 23:50:41]

tsepez@chromium.org changed reviewers:
+ bo_xu@foxitsoftware.com, jun_fang@foxitsoftware.com

[Tom Sepez, 2014-10-13 23:50:42]

Please review.  Just something that could be a problem somday.

[Bo Xu, 2014-10-14 00:17:23]

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
File core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp (right):

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:72: return result;
In all the lines above having "return m_pSyntax->GetPos()", also need to do
"m_pSyntax = NULL;"?

Or set return value at each location, break out and all return at the end of the
function?

[jun_fang, 2014-10-14 00:30:43]

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
File core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp (right):

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:43: while (1) {
FX_BOOL bContinue = TRUE;
while (bContinue) {

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:50: return
m_pSyntax->GetPos();
bContinue = FALSE;
break;
//return m_pSyntax->GetPos();

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:53: m_bAbort = TRUE;
 if(!OnOperator((char*)syntax.GetWordBuf()) &&
_PDF_HasInvalidOpChar((char*)syntax.GetWordBuf())) {
    m_bAbort = TRUE;
    bContinue = FALSE;
    break;
}
//if (m_bAbort) {
//    return m_pSyntax->GetPos();					
//}

https://codereview.chromium.org/652063002/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:71: m_pSyntax = NULL;
m_pSyntax = NULL;
return syntax.GetPos();
//return m_pSyntax->GetPos();

[Tom Sepez, 2014-10-14 16:54:06]

Ah, yes, good spotting there Bo and Jun, I'd missed those returns.  In cases
like this, its easier to let the c++ compiler take care of covering all exit
paths to avoid mistakes down the road. Ideally, auto-clearer / auto-restorer and
such would be templated and part of fx_basic ...

[jun_fang, 2014-10-14 20:49:42]

On 2014/10/14 16:54:06, Tom Sepez wrote:
> Ah, yes, good spotting there Bo and Jun, I'd missed those returns.  In cases
> like this, its easier to let the c++ compiler take care of covering all exit
> paths to avoid mistakes down the road. Ideally, auto-clearer / auto-restorer
and
> such would be templated and part of fx_basic ...

It's a perfect solution. LGTM

[jun_fang, 2014-10-14 20:54:08]

https://codereview.chromium.org/652063002/diff/80001/core/src/fpdfapi/fpdf_pa...
File core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp (right):

https://codereview.chromium.org/652063002/diff/80001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:38: *scoped_variable_ =
new_parser;
nit: shall we check whether scoped_variable_ is not NULL?

[Tom Sepez, 2014-10-14 21:23:02]

> nit: shall we check whether scoped_variable_ is not NULL?
I don't think we need to do this, since the usage so far is always &something
for scoped_variable.

[Tom Sepez, 2014-10-14 21:41:01]

Committed patchset #5 (id:80001) manually as
35d720aff01c5ea778c16ac1e31c56f68490f10b (presubmit successful).