Re: [PATCH 10/14] exynos/fimg2d: remove default case from g2d_get_blend_op()

[Tobias Jakobi <tjakobi@math.uni-bielefeld.de>]

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
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel