Hi Geert

Am 10.03.23 um 09:18 schrieb Geert Uytterhoeven:

Hi Thomas,

On Thu, Mar 9, 2023 at 5:02 PM Thomas Zimmermann [off-list ref] wrote:

quoted

Assume that the driver does not own the option string or its substrings
and hence duplicate the option string for the video mode. Allocate the
copy's memory with kstrdup() and free it in the module's exit function.

Done in preparation of switching the driver to struct option_iter and
constifying the option string.

v2:
         * replace static memory with kstrdup()/kfree() (Geert)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for the upodate!

quoted

--- a/drivers/video/fbdev/ps3fb.c
+++ b/drivers/video/fbdev/ps3fb.c

@@ -260,6 +260,7 @@ static const struct fb_videomode ps3fb_modedb[] = {
  static int ps3fb_mode;
  module_param(ps3fb_mode, int, 0);

+static char *mode_option_buf;

Do you really need this variable? It contains the same value
as mode_option below.
This is a common pattern in several patches.

In most cases, the extra variable is necessary. mode_option, or a 
similar variable, is often also used to store module parameters or is 
initialized from a static string. In those cases we obviously cannot 
free the memory. Hence the additional variable for the freeing the 
allocation.

I reviewed the patches and it looks like kyrofb, ps3fb, and sm712fb 
might be able to work without the _buf variable. I'll try to update those.

Best regards
Thomas

quoted

  static char *mode_option;

  static int ps3fb_cmp_mode(const struct fb_videomode *vmode,

@@ -1276,8 +1277,11 @@ static int __init ps3fb_setup(void)
                         continue;
                 if (!strncmp(this_opt, "mode:", 5))
                         ps3fb_mode = simple_strtoul(this_opt + 5, NULL, 0);
-               else
-                       mode_option = this_opt;
+               else {
+                       kfree(mode_option_buf);
+                       mode_option_buf = kstrdup(this_opt, GFP_KERNEL); // ignore errors
+                       mode_option = mode_option_buf;
+               }
         }
         return 0;
  }

@@ -1294,6 +1298,7 @@ static void __exit ps3fb_exit(void)
  {
         pr_debug(" -> %s:%d\n", __func__, __LINE__);
         ps3_system_bus_driver_unregister(&ps3fb_driver);
+       kfree(mode_option_buf);
         pr_debug(" <- %s:%d\n", __func__, __LINE__);
  }

Gr{oetje,eeting}s,

                         Geert

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev