Age | Commit message (Collapse) | Author |
|
FX_ArraySize macro.
pdfium has numerous places where the number of elements in an array is
calculated with expressions like:
sizeof(cFormats)/sizeof(FX_LPCWSTR)
This is suboptimal because it is verbose, it is easy to get wrong, and
it cannot be determined through casual inspection whether the code is
correct. It will give incorrect results if cFormats is a pointer instead
of an array and it will give incorrect results if FX_LPCWSTR is not the
type of the array elements.
The FX_WSTRC macro in fx_string.h which I fixed was particularly scary
because it would silently misbehave if passed a pointer.
The FX_ArraySize macro which I have added and started using (taken from
arraysize in v8's macros.h) is easier to use and will always give correct
results. If passed a pointer it will fail to compile.
For this change I only fixed instances of sizeof(FX_LPCWSTR). There
appear to be about 150 other places in the pdfium code that could
benefit from using FX_ArraySize.
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/729293003
|
|
Since casts to FX_LPCWSTR have been shown to hide bugs I tried removing
more of them, targeting those places where a cast was used to force a
conversion from CFX_WideString to FX_LPCWSTR, replacing these casts with
calls to the newly added .c_str() function. This revealed two places
where the cast was hiding a bug -- where ->c_str() was required instead!
This removes ~33 FX_LPCWSTR casts and there are ~31 left, many of which
will go away in some future change.
Also includes this change:
Removing unnecessary casts from wchar_t* to wchar_t*, by various names.
Original patch from Bruce Dawson(brucedawson@chromium.org)
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/733693003
|
|
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/775903002
|
|
BUG=429139,430566,431288
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/758593002
|
|
Before this fix, PDF parser aborts the parsering process when detecting an error.
For this case, PDF parser just gives up parsering when it detects that the length of
image stream is incorrect. The solution to this case is to find the tag "endstream"
and "endobj" to calculate the length rather than aborting the parsering process.
BUG=433339
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/743263002
|
|
BUG=433992
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/733273002
|
|
VC++'s /analyze points out that this expression:
FX_BOOL bCheckRight = type != 'D' || type != 'W';"
is always true. This means that the tests for the right edge of a word
Original patch from Bruce Dawson(brucedawson@chromium.org)
BUG=427616
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/731673003
|
|
As of the 2013 version VC++ still doesn't support the 'z' size specifier. This makes portable printing of size_t types frustrating. The simplest general solution is to use %u and cast to unsigned. If there was any possibility of the numbers getting larger than 32-bit then we would need better alternatives, but there is not.
This was found through code inspection, through /analyze, and through pdfium_test print this non-helpful message:
Loaded, parsed and rendered zu pages.
Skipped zu bad pages.
I can confirm that the fix works on Windows and it should work identically on mac. This is a follow-on to change 02e6ca4c4f.
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/738433003
|
|
Remove casts that merely cast from wchar_t* to wchar_t*. Sometimes the
types or casts are FX_LPCWSTR but the idea is the same. Excess casts
can (and have) hidden bugs so removing these may prevent future problems.
Original patch from Bruce Dawson(brucedawson@chromium.org)
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/730993002
|
|
Whether ch and iRet are read without being initialized depends on complex
preconditions and cannot be determined by looking at these function.
Therefore it seems prudent to zero initialize them to avoid any risk.
BUG=427616
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/727083002
|
|
BUG=https://code.google.com/p/pdfium/issues/detail?id=78
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/726143002
|
|
Error is "converting to non-pointer type 'FX_DWORD'".
TBR=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/726033002
|
|
BUG=401988
R=vitalybuka@chromium.org
Review URL: https://codereview.chromium.org/618073003
|
|
The flag variable is conditionally initialized but unconditionally read.
Warning was:
src\fpdfapi\fpdf_page\fpdf_page_pattern.cpp(274) : warning C6001:
Using uninitialized memory 'flag'.
BUG=427616
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/703213004
|
|
BUG=418582
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/680833006
|
|
BUG=414036, 425151
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/688633003
|
|
Add a check for zero-length keys to avoid hitting the equivalent of |""[1]|.
BUG=https://code.google.com/p/pdfium/issues/detail?id=58
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/665223003
|
|
BUG=418976, 425150, 414525
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/671943002
|
|
pTransfer function is released in pDocRender cleanup but is still being accessed in
~CPDF_GeneralStateData in pDocPage cleanup.
BUG=419320
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/662063003
|
|
BUG=414089, 414310, 414606
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/670813002
|
|
This patch is supplementary to issue 418881
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/645793007
|
|
You can get warning when using FXARGB_GETDIB() with & operation in
the same statement like: FXARGB_GETDIB(src_scan) & 0xffffff in
fx_dib_composite.cpp:
../../third_party/pdfium/core/src/fxge/dib/fx_dib_composite.cpp:737:205:
error: '&' within '|' [-Werror,-Wbitwise-op-parentheses]
Original patch from jiangj@opera.com
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/578253002
|
|
This is a re-landing of the changes in https://pdfium.googlesource.com/pdfium/+/6387aff
which were lost during a libopenjpeg library roll.
TBR=thestig@chromium.org
Review URL: https://codereview.chromium.org/661463003
|
|
CPDF_Pattern objects are counted and maintained in m_PatternedMap.
When a CPDF_Pattern object "pattern" is deleted, it's address is marked as NULL in m_PatternMap.
This patch stores the address of CPDF_Pattern's adderss in all objects that references "pattern",
to ensure valid referencing after deletion.
BUG=416319, 419976, 418392
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/656753002
|
|
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
Review URL: https://codereview.chromium.org/652063002
|
|
Glyph index shall be less than number of glyphs.
BUG=418585
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/652363002
|
|
BUG=421196
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/656463006
|
|
BUG=418881
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/648823002
|
|
BUG=413375
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/624023003
|
|
BUG=407964, 414182, 413447
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/589243004
|
|
BUG=408532
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/524443002
|
|
Images are basic resource and are referred or used by other objects in some cases. Images should be released after the objects who uses these objects. In this case, an image object is accessed in the process of CPDF_TilingPattern's destroy. Unlikely, this image has been destroyed before.
BUG=414046
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/582993002
|
|
This was introduced at PDFium revision 12a9940. There was a subtle logic
change for null |parray|.
BUG=415438
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/579363002
|
|
pattern
BUG=414661
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/572853006
|
|
Before this fix, only |gindex < face->num_locations| is checked. However, the pointer, p, will be moved to next location in the first FT_NEXT_ULONG or FT_NEXT_USHORT. It may cause a crashier. So a boundary check is needed before the second FT_NEXT_ULONG or FT_NEXT_USHORT.
BUG=412457
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/571833002
|
|
BUG=414155
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/574463002
|
|
There is an image object and text objects in this document, but the character in each text object is reversed.
When rendering, the image object is shown.
However, when highlighting, the text object is selected, resulting in text index issue.
Moreover, the character in the document is in reading order, which is different from normal document.
BUG=pdfium:43
R=jbreiden@google.com
Review URL: https://codereview.chromium.org/484503002
|
|
BUG=387970
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/551503003
|
|
m_pDocument in CPDF_Color
If not forced clear, the counter of CPDF_Pattern will take care the count of referenced instance.
When forced clear, clear color first to make sure pattern still exists when referencing it.
BUG=411154
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/554913003
|
|
BUG=411161
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/560673002
|
|
of https://codereview.chromium.org/547863002/)
Reason for revert:
The initialized member variable is wrong. Needs to undo this.
Original issue's description:
> Fix uninitialized value in CPDF_ColorStateData
>
> BUG=411161
> R=tsepez@chromium.org
>
> Committed: https://pdfium.googlesource.com/pdfium/+/a38d4c3572b566f8e64b9ca4f725cbbc581e85ab
TBR=tsepez@chromium.org,jun_fang@foxitsoftware.com
NOTREECHECKS=true
NOTRY=true
BUG=411161
Review URL: https://codereview.chromium.org/558803004
|
|
BUG=411161
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/547863002
|
|
BUG=411163
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/551513005
|
|
BUG=
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/534363002
|
|
When |m_nComponents| is changed from loading stream information,
previously allocated memory that depends on |m_nComponents| needes to be freed
and allocated again to enforce memory size consistency.
BUG=409695
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/528163002
|
|
BUG=409692
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/534763002
|
|
This will prevent using freed pattern object.
This is a better solution than https://pdfium.googlesource.com/pdfium/+/1b9c5c4dc41956b8c5ab17b9a882adf8a2513768
and in essence revert that patch
BUG=409373
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/522483003
|
|
This is code cleanup rather than bug fixing.
The motivation for this was to fix the casts at line 97 of the original file. These are wrong; you cannot correct via casting a function signature mismatch when passing a function as an argument. In theory, there's no reason to believe that the compiler will pass args in the same manner for a function of type (void*, size_t, void*) as for a function of type (void*, size_t, some_struct*). The cast will suppress the compile error, but you can't be assured the call will work as intended. In practice, it does, since the last architecture where a void* had a different representation than a struct* went extinct in the late 80s.
In the functions themselves, note that we currently bail out if srcData->offset >= srcData->src_size, so the expression
bufferLength = (OPJ_SIZE_T)(srcData->src_size - srcData->offset)
will always be > 0. Hence the check
if(bufferLength <= 0)
is pointless, esp. since bufferLength is a signed type and < 0 makes no sense.
The opj_seek_from_memory() has a bool return value, so returning -1 on error doesn't seem reasonable. Change this to TRUE/FALSE, and return false on seek past end.
If we're truly passing readonly data, then perhaps it makes sense to make the write() function always return -1. I didn't do this.
Lastly, I capitalize "DecodeData" so that it looks like a struct, and change its members to be size_t's to avoid casting back and forth.
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/507273003
|
|
m_bpc is assigned.
The problem of using GetValidBpc() in each function call is it could result in mismatch as seen in this case:
in ContinueToLoadMask(), m_bpc is re-assigned to 1 if m_bImageMask==1 regardless of the value from GetValidBpc().
This will result in mismatch if another function use the value from GetValidBpc().
The solution could be checking m_bImageMask in another function to make sure m_bpc is consistent, but that makes the code too cumbersome.
Also, we have to bring and are bringing in more and more GetValidBpc check, and this will continue with other buggy documents. So better to fix it now.
The original rational to use GetValidBpc() in where m_bpc is used is to respect the "raw" data from parsing.
However, if it will be ignored anyway and using value from GetValidBpc(), we'd better correct it at the very beginning.
BUG=408541
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/518443002
|
|
Edge closer to getting PDFium to build in chromium_code mode.
BUG=https://code.google.com/p/pdfium/issues/detail?id=29
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/437343002
|