Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(374)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1549073002: Fix crash from commit f6dafc9. (Closed)

Created:
4 years, 12 months ago by Lei Zhang
Modified:
4 years, 12 months ago
Reviewers:
Wei Li
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp View 2 chunks +2 lines, -2 lines 2 comments Download

Messages

Total messages: 5 (1 generated)
Lei Zhang
TBR Will add a regression test in a separate CL.
4 years, 12 months ago (2015-12-24 10:21:43 UTC) #1
Lei Zhang
Committed patchset #1 (id:1) manually as 56b56a86842e3e8f81dfefab994cca8934a3c4d4 (presubmit successful).
4 years, 12 months ago (2015-12-24 10:22:54 UTC) #3
Wei Li
lgtm https://codereview.chromium.org/1549073002/diff/1/core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right): https://codereview.chromium.org/1549073002/diff/1/core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp#newcode1322 core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:1322: FX_DWORD nLastObjNum = m_pParser->GetLastObjNum(); Need to check here ...
4 years, 12 months ago (2015-12-24 18:09:15 UTC) #4
Lei Zhang
4 years, 12 months ago (2015-12-24 18:53:42 UTC) #5
Message was sent while issue was closed. 
https://codereview.chromium.org/1549073002/diff/1/core/src/fpdfapi/fpdf_edit/...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1549073002/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:1322: FX_DWORD nLastObjNum =
m_pParser->GetLastObjNum();
On 2015/12/24 18:09:14, Wei Li(ooo) wrote:
> Need to check here too?

It was already dereferencing |m_pParser| without a check before my CL, so I'm
leaving it as is.

Powered by Google App Engine
This is Rietveld 408576698