On 31 August 2015 at 14:18, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
> On 2015년 08월 24일 23:14, Tobias Jakobi wrote:
>> The G2D headers define a number of modes through enums
>> (like e.g. color, select, repeat, etc.).
>>
>> This introduces g2d_validate_select_mode() and
>> g2d_validate_blending_op() which validate a
>> select mode or blending operation respectively.
>>
>> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx>
>> ---
>> exynos/exynos_fimg2d.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
>> index 2e04f4a..781aff5 100644
>> --- a/exynos/exynos_fimg2d.c
>> +++ b/exynos/exynos_fimg2d.c
>> @@ -119,6 +119,50 @@ static unsigned int g2d_check_space(const struct g2d_context *ctx,
>> }
>>
>> /*
>> + * g2d_validate_select_mode - validate select mode.
>> + *
>> + * @mode: the mode to validate
>> + */
>> +static unsigned int g2d_validate_select_mode(
>> + enum e_g2d_select_mode mode)
>> +{
>> + switch (mode) {
>> + case G2D_SELECT_MODE_NORMAL:
>> + case G2D_SELECT_MODE_FGCOLOR:
>> + case G2D_SELECT_MODE_BGCOLOR:
>> + return 0;
>> + }
>
> It's strange use a bit. Just check the range like below,
>
> First, please add G2D_SELECT_MODE_MAX which has 3 << 0, and
>
> if (G2D_SELECT_MODE_MAX < mode) {
> fprintf(strerr, "invalid command(0x%x)\n", mode);
> return -EINVAL;
> }
>
Listing every value might seem like an overkill, indeed. Yet I think
it's the more robust approach.
By adding _MAX to the API we effectively lock it down. That can be
avoided, plus the compiler will warn us when new values are added to
the enum. If someone starts getting smart (due to the _MAX) and adds
G2D_SELECT_MODE_FOO = -1, the above check will fail :(
> And I think it'd be better to change return type of this function to int,
>
Good idea.
Cheers,
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
