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

Issue 826573003: Fixed incorrect use of FX_WSTRC on FX_WCHAR* vars. (Closed)

Created:
6 years ago by brucedawson
Modified:
5 years, 11 months ago
Reviewers:
Bo Xu
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@xfa
Target Ref:
refs/heads/xfa
Visibility:
Public.

Description

Fixed incorrect use of FX_WSTRC on FX_WCHAR* vars. FX_WSTRC is only valid on arrays, not pointers. In five places it was being passed a pointer, which leads to incorrect string objects being created. This was found when integrating a change to FX_WSTRC that disallows pointer arguments. The consequence of this bug is that five XML strings (quot, amp, apos, lt, and gt) will all end up with incorrect lengths. They will all be one character long in 32-bit builds, and three characters long in 64-bit builds (sizeof(WCHAR*)-1). Also removed some unneeded casts and marked some arrays as const. Fixing this is necessary in order to allow landing of https://codereview.chromium.org/818193004/ Testing this was attempted by using the xfa branch of pdfium in Chrome: cd third_party\pdfium git checkout xfa However even without these changes this caused a CHECK failure in V8::InitializePlatform due to double initialization, so the fix has not been tested, but is clearly an improvement. BUG= https://code.google.com/p/pdfium/issues/detail?id=96 R=bo_xu@foxitsoftware.com Committed: https://pdfium.googlesource.com/pdfium/+/fef49bc5e23e860fc9d3529839d1a4eb684feafc

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -19 lines) Patch
M xfa/src/fxfa/src/fm2js/xfa_fm2jscontext.cpp View 4 chunks +19 lines, -19 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
brucedawson
6 years ago (2014-12-23 22:18:49 UTC) #1
brucedawson
I still haven't been able to test this, but I think pushing the change through ...
5 years, 11 months ago (2014-12-29 20:31:56 UTC) #2
brucedawson
How does this look? I haven't managed to test it but it seems clear that ...
5 years, 11 months ago (2015-01-02 21:18:25 UTC) #3
Bo Xu
On 2015/01/02 21:18:25, brucedawson wrote: > How does this look? I haven't managed to test ...
5 years, 11 months ago (2015-01-02 21:51:48 UTC) #4
brucedawson
5 years, 11 months ago (2015-01-05 18:06:52 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
fef49bc5e23e860fc9d3529839d1a4eb684feafc (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698