| Message ID | 1440425649-9768-11-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2015? 08? 24? 23:14, Tobias Jakobi wrote: > We now validate the blending mode via g2d_validate_mode() > prior to feeding it to g2d_get_blend_op(). > > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > exynos/exynos_fimg2d.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c > index 4274a94..d708e91 100644 > --- a/exynos/exynos_fimg2d.c > +++ b/exynos/exynos_fimg2d.c > @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) > SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, > G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); > break; > - default: > - fprintf(stderr, "Not support operation(%d).\n", op); > - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, > - 0, 0, 0); > - break; With this, how about changing above switch and case statement to if statement? Thanks, Inki Dae > } > > return val.val; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 August 2015 at 14:25, Inki Dae <inki.dae@samsung.com> wrote: > On 2015? 08? 24? 23:14, Tobias Jakobi wrote: >> We now validate the blending mode via g2d_validate_mode() >> prior to feeding it to g2d_get_blend_op(). >> >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >> --- >> exynos/exynos_fimg2d.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >> index 4274a94..d708e91 100644 >> --- a/exynos/exynos_fimg2d.c >> +++ b/exynos/exynos_fimg2d.c >> @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) >> SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, >> G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); >> break; >> - default: >> - fprintf(stderr, "Not support operation(%d).\n", op); >> - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, >> - 0, 0, 0); >> - break; > > With this, how about changing above switch and case statement to if > statement? > Out of curiosity: why is if/else statement preferred - won't it make things longer/harder to read (there are 11 cases here) ? Cheers, Emil -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015? 09? 01? 03:53, Emil Velikov wrote: > On 31 August 2015 at 14:25, Inki Dae <inki.dae@samsung.com> wrote: >> On 2015? 08? 24? 23:14, Tobias Jakobi wrote: >>> We now validate the blending mode via g2d_validate_mode() >>> prior to feeding it to g2d_get_blend_op(). >>> >>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>> --- >>> exynos/exynos_fimg2d.c | 5 ----- >>> 1 file changed, 5 deletions(-) >>> >>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >>> index 4274a94..d708e91 100644 >>> --- a/exynos/exynos_fimg2d.c >>> +++ b/exynos/exynos_fimg2d.c >>> @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) >>> SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, >>> G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); >>> break; >>> - default: >>> - fprintf(stderr, "Not support operation(%d).\n", op); >>> - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, >>> - 0, 0, 0); >>> - break; >> >> With this, how about changing above switch and case statement to if >> statement? >> > Out of curiosity: why is if/else statement preferred - won't it make > things longer/harder to read (there are 11 cases here) ? Just looks strange to me switch and case statement has no default statement. This is just my opinion and trivial. Thanks, Inki Dae > > Cheers, > Emil > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Inki, Inki Dae wrote: > On 2015? 09? 01? 03:53, Emil Velikov wrote: >> On 31 August 2015 at 14:25, Inki Dae <inki.dae@samsung.com> wrote: >>> On 2015? 08? 24? 23:14, Tobias Jakobi wrote: >>>> We now validate the blending mode via g2d_validate_mode() >>>> prior to feeding it to g2d_get_blend_op(). >>>> >>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>> --- >>>> exynos/exynos_fimg2d.c | 5 ----- >>>> 1 file changed, 5 deletions(-) >>>> >>>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >>>> index 4274a94..d708e91 100644 >>>> --- a/exynos/exynos_fimg2d.c >>>> +++ b/exynos/exynos_fimg2d.c >>>> @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) >>>> SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, >>>> G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); >>>> break; >>>> - default: >>>> - fprintf(stderr, "Not support operation(%d).\n", op); >>>> - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, >>>> - 0, 0, 0); >>>> - break; >>> >>> With this, how about changing above switch and case statement to if >>> statement? >>> >> Out of curiosity: why is if/else statement preferred - won't it make >> things longer/harder to read (there are 11 cases here) ? > > Just looks strange to me switch and case statement has no default > statement. This is just my opinion and trivial. I would like to keep the switch statement here. As Emil has pointed out the big advantage of switch is that the compiler can warn us if we're not dealing with a value of an enum. I think that's definitely a feature which we want here. I would suggest instead this: I add a short comment to the switch making clear why we don't have a default case here and that a user of g2d_get_blend_op() should first validate any data passed to it. With best wishes, Tobias > > Thanks, > Inki Dae > >> >> Cheers, >> Emil >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c index 4274a94..d708e91 100644 --- a/exynos/exynos_fimg2d.c +++ b/exynos/exynos_fimg2d.c @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); break; - default: - fprintf(stderr, "Not support operation(%d).\n", op); - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, - 0, 0, 0); - break; } return val.val;
We now validate the blending mode via g2d_validate_mode() prior to feeding it to g2d_get_blend_op(). Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> --- exynos/exynos_fimg2d.c | 5 ----- 1 file changed, 5 deletions(-)