Add more |bpc| value check in GetValidBpc()

Add more |bpc| value check in GetValidBpc()

BUG=382988
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/90d1f9b

[Bo Xu, 2014-08-01 20:19:13]

Please help review this.

[Tom Sepez, 2014-08-01 20:33:53]

LGTM with optional nits.

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:938: if (bpc == 16) {
nit: someday down the road, this might be better served by a switch statement.

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:946: }
nit: this indentation style doesn't match what's in-use throughout the rest of
the file. Although it may be clearer the way you've written it, I'd encourage
you do it consistently with the existing convention for consistency.

[Bo Xu, 2014-08-01 21:03:01]

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:938: if (bpc == 16) {
On 2014/08/01 20:33:53, Tom Sepez wrote:
> nit: someday down the road, this might be better served by a switch statement.

Changed to switch statement.

https://codereview.chromium.org/430733004/diff/1/core/src/fpdfapi/fpdf_render...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:946: }
On 2014/08/01 20:33:53, Tom Sepez wrote:
> nit: this indentation style doesn't match what's in-use throughout the rest of
> the file. Although it may be clearer the way you've written it, I'd encourage
> you do it consistently with the existing convention for consistency.

Done.

[Tom Sepez, 2014-08-01 21:10:48]

Still LGTM with nits.

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:939: switch (bpc){
nit: space after )

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:973: }
nit: some picky people might insist on a break here.

[Bo Xu, 2014-08-01 23:13:52]

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:939: switch (bpc){
On 2014/08/01 21:10:48, Tom Sepez wrote:
> nit: space after )

Done.

https://codereview.chromium.org/430733004/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:973: }
On 2014/08/01 21:10:48, Tom Sepez wrote:
> nit: some picky people might insist on a break here.

Done.

[Bo Xu, 2014-08-01 23:14:33]

Committed patchset #3 manually as 90d1f9b (presubmit successful).