|
|
DescriptionClean up fx_ge_linux.cpp a little.
Committed: https://pdfium.googlesource.com/pdfium/+/611597203cc5fda87717fcd350b9b5932320e3c0
Patch Set 1 #
Total comments: 4
Messages
Total messages: 16 (9 generated)
The CQ bit was checked by thestig@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
thestig@chromium.org changed reviewers: + npm@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
npm@google.com changed reviewers: + npm@google.com
lgtm only one comment https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cpp File core/fxge/ge/fx_ge_linux.cpp (right): https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cp... core/fxge/ge/fx_ge_linux.cpp:101: ASSERT(index < FX_ArraySize(g_LinuxGpFontList)); Maybe also include the condition index >= 0
https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cpp File core/fxge/ge/fx_ge_linux.cpp (right): https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cp... core/fxge/ge/fx_ge_linux.cpp:101: ASSERT(index < FX_ArraySize(g_LinuxGpFontList)); On 2016/08/04 17:53:16, npm_g wrote: > Maybe also include the condition index >= 0 size_t is unsigned, always >= 0.
The CQ bit was checked by thestig@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Clean up fx_ge_linux.cpp a little. ========== to ========== Clean up fx_ge_linux.cpp a little. Committed: https://pdfium.googlesource.com/pdfium/+/611597203cc5fda87717fcd350b9b5932320... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://pdfium.googlesource.com/pdfium/+/611597203cc5fda87717fcd350b9b5932320...
Message was sent while issue was closed.
tsepez@chromium.org changed reviewers: + tsepez@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cpp File core/fxge/ge/fx_ge_linux.cpp (right): https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cp... core/fxge/ge/fx_ge_linux.cpp:35: const FX_CHAR* NameArr[kLinuxGpNameSize]; drive-by: wouldn't a 2D array of char* be simpler than an array of struct containing an array of char* ??
Message was sent while issue was closed.
https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cpp File core/fxge/ge/fx_ge_linux.cpp (right): https://codereview.chromium.org/2218433002/diff/1/core/fxge/ge/fx_ge_linux.cp... core/fxge/ge/fx_ge_linux.cpp:35: const FX_CHAR* NameArr[kLinuxGpNameSize]; On 2016/08/04 19:20:18, Tom Sepez wrote: > drive-by: wouldn't a 2D array of char* be simpler than an array of struct > containing an array of char* ?? Probably. Who wants to send the next CL? :) |