From 7a052545d507d53574a295432db0923a1b50eda7 Mon Sep 17 00:00:00 2001 From: Pisit Sawangvonganan Date: Wed, 6 Nov 2024 14:11:13 +0700 Subject: [PATCH 1/4] fb: cfb: avoid multiple `strlen` calls in `draw_text` Added `len` to store the result of `strlen(str)` to avoid multiple calls to `strlen` in the `for-loop`. Signed-off-by: Pisit Sawangvonganan --- subsys/fb/cfb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/subsys/fb/cfb.c b/subsys/fb/cfb.c index 7dc3a3b5cfc91..3cf5db2c0b064 100644 --- a/subsys/fb/cfb.c +++ b/subsys/fb/cfb.c @@ -278,7 +278,9 @@ static int draw_text(const struct device *dev, const char *const str, int16_t x, } if ((fb->screen_info & SCREEN_INFO_MONO_VTILED)) { - for (size_t i = 0; i < strlen(str); i++) { + const size_t len = strlen(str); + + for (size_t i = 0; i < len; i++) { if ((x + fptr->width > fb->x_res) && wrap) { x = 0U; y += fptr->height; From f836f202d1d03184252b355e4b05af3f5303b20a Mon Sep 17 00:00:00 2001 From: Pisit Sawangvonganan Date: Wed, 6 Nov 2024 14:21:33 +0700 Subject: [PATCH 2/4] fb: cfb: remove unnecessary `NULL` check and `NULL` assignment Since the `fb` pointer is always assigned to `char_fb`, there is no need for a `NULL` check. Additionally, removed setting `fb->buf` to `NULL` in `cfb_framebuffer_init` as it will be overwritten by subsequent operations. Signed-off-by: Pisit Sawangvonganan --- subsys/fb/cfb.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/subsys/fb/cfb.c b/subsys/fb/cfb.c index 3cf5db2c0b064..b237a3d8386fd 100644 --- a/subsys/fb/cfb.c +++ b/subsys/fb/cfb.c @@ -436,7 +436,7 @@ int cfb_framebuffer_clear(const struct device *dev, bool clear_display) { const struct char_framebuffer *fb = &char_fb; - if (!fb || !fb->buf) { + if (!fb->buf) { return -ENODEV; } @@ -465,7 +465,7 @@ int cfb_framebuffer_finalize(const struct device *dev) struct display_buffer_descriptor desc; int err; - if (!fb || !fb->buf) { + if (!fb->buf) { return -ENODEV; } @@ -574,7 +574,6 @@ int cfb_framebuffer_init(const struct device *dev) fb->ppt = 8U; fb->pixel_format = cfg.current_pixel_format; fb->screen_info = cfg.screen_info; - fb->buf = NULL; fb->kerning = 0; fb->inverted = false; From 4e4f80e9690f56167b3aee1b0312f2c168cbb78c Mon Sep 17 00:00:00 2001 From: Pisit Sawangvonganan Date: Wed, 6 Nov 2024 14:40:05 +0700 Subject: [PATCH 3/4] fb: cfb_shell: use `shell_strtol` in `cmd_set_kerning` Switch from using direct `strtol` calls to `shell_strtol`. This change leverages the extensive error handling provided by `shell_strtol`. Signed-off-by: Pisit Sawangvonganan --- subsys/fb/cfb_shell.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/subsys/fb/cfb_shell.c b/subsys/fb/cfb_shell.c index b6d246f78fb39..fb8cf7447676e 100644 --- a/subsys/fb/cfb_shell.c +++ b/subsys/fb/cfb_shell.c @@ -340,8 +340,7 @@ static int cmd_set_font(const struct shell *sh, size_t argc, char *argv[]) static int cmd_set_kerning(const struct shell *sh, size_t argc, char *argv[]) { - int err; - char *ep = NULL; + int err = 0; long kerning; if (!dev) { @@ -349,9 +348,8 @@ static int cmd_set_kerning(const struct shell *sh, size_t argc, char *argv[]) return -ENODEV; } - errno = 0; - kerning = strtol(argv[1], &ep, 10); - if (errno || ep == argv[1]) { + kerning = shell_strtol(argv[1], 10, &err); + if (err) { shell_error(sh, HELP_INIT); return -EINVAL; } From 55bfc8128485b4c8ff4822666106adcda22094fe Mon Sep 17 00:00:00 2001 From: Pisit Sawangvonganan Date: Wed, 6 Nov 2024 14:55:10 +0700 Subject: [PATCH 4/4] fb: cfb_shell: remove `dev` null check Remove `dev` null check as `DEVICE_DT_GET` ensures compile-time initialization. Refer to commit 6eb371f (fb: initialize devices at compile time) Signed-off-by: Pisit Sawangvonganan --- subsys/fb/cfb_shell.c | 100 ------------------------------------------ 1 file changed, 100 deletions(-) diff --git a/subsys/fb/cfb_shell.c b/subsys/fb/cfb_shell.c index fb8cf7447676e..571c4dc3c582b 100644 --- a/subsys/fb/cfb_shell.c +++ b/subsys/fb/cfb_shell.c @@ -35,11 +35,6 @@ static int cmd_clear(const struct shell *sh, size_t argc, char *argv[]) ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - err = cfb_framebuffer_clear(dev, true); if (err) { shell_error(sh, "Framebuffer clear error=%d", err); @@ -62,11 +57,6 @@ static int cmd_cfb_print(const struct shell *sh, int col, int row, char *str) int err; uint8_t ppt; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - ppt = cfb_get_display_parameter(dev, CFB_DISPLAY_PPT); err = cfb_framebuffer_clear(dev, false); @@ -97,11 +87,6 @@ static int cmd_print(const struct shell *sh, size_t argc, char *argv[]) int err; int col, row; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - col = strtol(argv[1], NULL, 10); if (col > cfb_get_display_parameter(dev, CFB_DISPLAY_COLS)) { shell_error(sh, "Invalid col=%d position", col); @@ -128,11 +113,6 @@ static int cmd_draw_text(const struct shell *sh, size_t argc, char *argv[]) int err; int x, y; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - x = strtol(argv[1], NULL, 10); y = strtol(argv[2], NULL, 10); err = cfb_draw_text(dev, argv[3], x, y); @@ -151,11 +131,6 @@ static int cmd_draw_point(const struct shell *sh, size_t argc, char *argv[]) int err; struct cfb_position pos; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - pos.x = strtol(argv[1], NULL, 10); pos.y = strtol(argv[2], NULL, 10); @@ -175,11 +150,6 @@ static int cmd_draw_line(const struct shell *sh, size_t argc, char *argv[]) int err; struct cfb_position start, end; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - start.x = strtol(argv[1], NULL, 10); start.y = strtol(argv[2], NULL, 10); end.x = strtol(argv[3], NULL, 10); @@ -201,11 +171,6 @@ static int cmd_draw_rect(const struct shell *sh, size_t argc, char *argv[]) int err; struct cfb_position start, end; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - start.x = strtol(argv[1], NULL, 10); start.y = strtol(argv[2], NULL, 10); end.x = strtol(argv[3], NULL, 10); @@ -228,11 +193,6 @@ static int cmd_scroll_vert(const struct shell *sh, size_t argc, char *argv[]) int col, row; int boundary; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - col = strtol(argv[1], NULL, 10); if (col > cfb_get_display_parameter(dev, CFB_DISPLAY_COLS)) { shell_error(sh, "Invalid col=%d position", col); @@ -269,11 +229,6 @@ static int cmd_scroll_horz(const struct shell *sh, size_t argc, char *argv[]) int col, row; int boundary; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - col = strtol(argv[1], NULL, 10); if (col > cfb_get_display_parameter(dev, CFB_DISPLAY_COLS)) { shell_error(sh, "Invalid col=%d position", col); @@ -312,11 +267,6 @@ static int cmd_set_font(const struct shell *sh, size_t argc, char *argv[]) uint8_t height; uint8_t width; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - idx = strtol(argv[1], NULL, 10); err = cfb_get_font_size(dev, idx, &width, &height); @@ -343,11 +293,6 @@ static int cmd_set_kerning(const struct shell *sh, size_t argc, char *argv[]) int err = 0; long kerning; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - kerning = shell_strtol(argv[1], 10, &err); if (err) { shell_error(sh, HELP_INIT); @@ -367,11 +312,6 @@ static int cmd_invert(const struct shell *sh, size_t argc, char *argv[]) { int err; - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - if (argc == 1) { err = cfb_framebuffer_invert(dev); if (err) { @@ -412,11 +352,6 @@ static int cmd_get_fonts(const struct shell *sh, size_t argc, char *argv[]) ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - for (int idx = 0; idx < cfb_get_numof_fonts(dev); idx++) { if (cfb_get_font_size(dev, idx, &font_width, &font_height)) { break; @@ -435,11 +370,6 @@ static int cmd_get_device(const struct shell *sh, size_t argc, char *argv[]) ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "Framebuffer Device: %s", dev->name); return err; @@ -451,11 +381,6 @@ static int cmd_get_param_all(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - for (unsigned int i = 0; i <= CFB_DISPLAY_COLS; i++) { shell_print(sh, "param: %s=%d", param_name[i], cfb_get_display_parameter(dev, i)); @@ -471,11 +396,6 @@ static int cmd_get_param_height(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "param: %s=%d", param_name[CFB_DISPLAY_HEIGH], cfb_get_display_parameter(dev, CFB_DISPLAY_HEIGH)); @@ -488,11 +408,6 @@ static int cmd_get_param_width(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "param: %s=%d", param_name[CFB_DISPLAY_WIDTH], cfb_get_display_parameter(dev, CFB_DISPLAY_WIDTH)); @@ -505,11 +420,6 @@ static int cmd_get_param_ppt(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "param: %s=%d", param_name[CFB_DISPLAY_PPT], cfb_get_display_parameter(dev, CFB_DISPLAY_PPT)); @@ -522,11 +432,6 @@ static int cmd_get_param_rows(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "param: %s=%d", param_name[CFB_DISPLAY_ROWS], cfb_get_display_parameter(dev, CFB_DISPLAY_ROWS)); @@ -539,11 +444,6 @@ static int cmd_get_param_cols(const struct shell *sh, size_t argc, ARG_UNUSED(argc); ARG_UNUSED(argv); - if (!dev) { - shell_error(sh, HELP_INIT); - return -ENODEV; - } - shell_print(sh, "param: %s=%d", param_name[CFB_DISPLAY_COLS], cfb_get_display_parameter(dev, CFB_DISPLAY_COLS));