Hello,
Emil Velikov wrote:
On 31 August 2015 at 14:18, Inki Dae inki.dae@samsung.com 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@math.uni-bielefeld.de
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.
that's my thinking as well. The overhead shouldn't be too high and the compiler probably optimizes this anyway.
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.
What would be the benefit of this? We're just returning only 0 and 1 anyway. My first reaction was to use a bool here.
Cheers, Emil
With best wishes, Tobias