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

Issue 551503003: Check object type for objects used in the dictory of 'Index' (Closed)

Created:
6 years, 3 months ago by jun_fang
Modified:
5 years, 9 months ago
Reviewers:
Tom Sepez, Bo Xu
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Visibility:
Public.

Description

Check object type for objects used in the dictory of 'Index' BUG=387970 R=tsepez@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/12a9940905825e028bebc0bf8d3eddcc65b1933d

Patch Set 1 #

Total comments: 4

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Total comments: 2

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -29 lines) Patch
M core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp View 1 2 3 4 5 2 chunks +40 lines, -29 lines 0 comments Download

Messages

Total messages: 10 (1 generated)
jun_fang
Hi guys, please review this change. Thanks!
6 years, 3 months ago (2014-09-07 05:49:44 UTC) #2
Tom Sepez
https://codereview.chromium.org/551503003/diff/1/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right): https://codereview.chromium.org/551503003/diff/1/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#newcode1038 core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:1038: if (pObj && (pObj->GetType() == PDFOBJ_NUMBER)) { nit: no ...
6 years, 3 months ago (2014-09-08 17:56:18 UTC) #3
jun_fang
please review it again. Thanks! https://codereview.chromium.org/551503003/diff/1/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right): https://codereview.chromium.org/551503003/diff/1/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#newcode1038 core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:1038: if (pObj && (pObj->GetType() ...
6 years, 3 months ago (2014-09-09 18:51:25 UTC) #4
Tom Sepez
https://codereview.chromium.org/551503003/diff/20001/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right): https://codereview.chromium.org/551503003/diff/20001/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#newcode1037 core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:1037: CPDF_Object* pObj = pArray->GetElement(i); Sorry, I'd be happier if ...
6 years, 3 months ago (2014-09-09 23:53:46 UTC) #5
jun_fang
On 2014/09/09 23:53:46, Tom Sepez wrote: > https://codereview.chromium.org/551503003/diff/20001/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp > File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right): > > https://codereview.chromium.org/551503003/diff/20001/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#newcode1037 ...
6 years, 3 months ago (2014-09-10 21:57:35 UTC) #6
Tom Sepez
Very nice. LGTM after fixing one nit. https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right): https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#newcode13 core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:13: using namespace ...
6 years, 3 months ago (2014-09-10 22:58:55 UTC) #7
Tom Sepez
https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (left): https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#oldcode12 core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:12: #include <limits.h> #include <utility> to be explicit about the ...
6 years, 3 months ago (2014-09-10 23:05:30 UTC) #8
jun_fang
On 2014/09/10 23:05:30, Tom Sepez wrote: > https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp > File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (left): > > https://codereview.chromium.org/551503003/diff/40002/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp#oldcode12 ...
6 years, 3 months ago (2014-09-10 23:11:59 UTC) #9
jun_fang
6 years, 3 months ago (2014-09-10 23:19:47 UTC) #10
Message was sent while issue was closed.
Committed patchset #6 (id:80001) manually as 12a9940 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698