Hello Heiko Schocher,
The patch 4295f9bf74a8: "video, sm501: add OF binding to support
SM501" from Jan 26, 2011, leads to the following static checker
warning:
drivers/video/fbdev/sm501fb.c:1958 sm501fb_probe()
warn: strcpy() 'cp' of unknown size might be too large for 'fb_mode'
drivers/video/fbdev/sm501fb.c
46 static char *fb_mode = "640x480-16@60";
[ snip ]
1953 info->pdata = &sm501fb_def_pdata;
1954 if (np) {
1955 /* Get EDID */
1956 cp = of_get_property(np, "mode", &len);
1957 if (cp)
1958 strcpy(fb_mode, cp);
I don't know anything about this hardware but there might be a mode
longer than "640x480-16@60"?
1959 prop = of_get_property(np, "edid", &len);
1960 if (prop && len = EDID_LENGTH) {
regards,
dan carpenter
Hello Dan,
Am 10.06.2015 17:51, schrieb Dan Carpenter:
Hello Heiko Schocher,
The patch 4295f9bf74a8: "video, sm501: add OF binding to support
SM501" from Jan 26, 2011, leads to the following static checker
warning:
drivers/video/fbdev/sm501fb.c:1958 sm501fb_probe()
warn: strcpy() 'cp' of unknown size might be too large for 'fb_mode'
drivers/video/fbdev/sm501fb.c
46 static char *fb_mode = "640x480-16@60";
[ snip ]
1953 info->pdata = &sm501fb_def_pdata;
1954 if (np) {
1955 /* Get EDID */
1956 cp = of_get_property(np, "mode", &len);
1957 if (cp)
1958 strcpy(fb_mode, cp);
I don't know anything about this hardware but there might be a mode
longer than "640x480-16@60"?
Hmm... I just used it with this resolution, but you are right, the
datasheet says "200 MHz DAC support 1280×1024 resolution" ... so, in
this case fb_mode would be to short ... I propose such a fix:
$ git diff
diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index 9e74e8f..2e57f2e 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -43,7 +43,9 @@
#include "edid.h"
-static char *fb_mode = "640x480-16@60";
+static char *fb_default_mode = "640x480-16@60";
+static char fb_mode[20];
+static char *fb_mode_cmdline;
static unsigned long default_bpp = 16;
static struct fb_videomode sm501_default_mode = {@@ -1963,6 +1965,11 @@ static int sm501fb_probe(struct platform_device *pdev)
if (info->edid_data)
found = 1;
}
+ } else {
+ if (fb_mode_cmdline)
+ strcpy(fb_mode, fb_mode_cmdline);
+ else
+ strcpy(fb_mode, fb_default_mode);
}
#endif
if (!found) {@@ -2230,7 +2237,7 @@ static struct platform_driver sm501fb_driver = {
module_platform_driver(sm501fb_driver);
-module_param_named(mode, fb_mode, charp, 0);
+module_param_named(mode, fb_mode_cmdline, charp, 0);
MODULE_PARM_DESC(mode,
"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
module_param_named(bpp, default_bpp, ulong, 0);
What do you think? If this is OK from your side, I can post this patch
"officially" ...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
On Thu, Jun 11, 2015 at 11:24:41AM +0200, Heiko Schocher wrote:
quoted hunk
static struct fb_videomode sm501_default_mode = {@@ -1963,6 +1965,11 @@ static int sm501fb_probe(struct platform_device *pdev)
if (info->edid_data)
found = 1;
}
+ } else {
+ if (fb_mode_cmdline)
+ strcpy(fb_mode, fb_mode_cmdline);
Could you make this a strncpy()? These days strlcpy() is
unfashionable... It's sort of pedantic and it goes over the 80 char
limit but the my static checker will complain if we don't.
strncpy(fb_mod, fb_mode_cmdline,
sizeof(fb_mod) - 1);
+ else
+ strcpy(fb_mode, fb_default_mode);
regards,
dan carpenter