-----Original Message-----
From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
owner@vger.kernel.org] On Behalf Of Dan Carpenter
Sent: Saturday, March 19, 2011 10:07 AM
To: Paul Mundt
Cc: Guennadi Liakhovetski; Magnus Damm; linux-fbdev@vger.kernel.org;
kernel-janitors@vger.kernel.org
Subject: [patch] fbdev: sh_mobile_lcdc: checking NULL instead of IS_ERR()
backlight_device_register() returns an ERR_PTR. It doesn't return NULL.
The patch is not applying on the master branch of fbdev tree.
I could find another branch: fbdev/shmobile on the tree.
It is a good idea to mention this in the description of the patch.
quoted hunk
Signed-off-by: Dan Carpenter <redacted>
diff --git a/drivers/video/sh_mobile_lcdcfb.c
b/drivers/video/sh_mobile_lcdcfb.c
index bf2629f..a53abe1 100644
*sh_mobile_lcdc_bl_probe(struct device *parent,
bl = backlight_device_register(ch->cfg.bl_info.name, parent, ch,
&sh_mobile_lcdc_bl_ops, NULL);
- if (!bl) {
+ if (IS_ERR(bl)) {
dev_err(parent, "unable to register backlight device\n");
How about printing the error number here?
return NULL;
Code is not checking for return value where this function is called.
A code snippet where this function is called:
/* probe the backlight is there is one defined */
if (ch->cfg.bl_info.max_brightness)
ch->bl = sh_mobile_lcdc_bl_probe(&pdev->dev, ch);
If the return value is not checked then whats the use of return value?
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Dan Carpenter <hidden> Date: 2011-03-21 04:39:25
On Mon, Mar 21, 2011 at 09:47:50AM +0530, Janorkar, Mayuresh wrote:
quoted
backlight_device_register() returns an ERR_PTR. It doesn't return NULL.
The patch is not applying on the master branch of fbdev tree.
I could find another branch: fbdev/shmobile on the tree.
It is a good idea to mention this in the description of the patch.
Sorry, I'm working against linux-next so I wasn't aware.
quoted
Signed-off-by: Dan Carpenter <redacted>
diff --git a/drivers/video/sh_mobile_lcdcfb.c
b/drivers/video/sh_mobile_lcdcfb.c
index bf2629f..a53abe1 100644
*sh_mobile_lcdc_bl_probe(struct device *parent,
bl = backlight_device_register(ch->cfg.bl_info.name, parent, ch,
&sh_mobile_lcdc_bl_ops, NULL);
- if (!bl) {
+ if (IS_ERR(bl)) {
dev_err(parent, "unable to register backlight device\n");
How about printing the error number here?
Ok. That's a good idea.
quoted
return NULL;
Code is not checking for return value where this function is called.
A code snippet where this function is called:
/* probe the backlight is there is one defined */
if (ch->cfg.bl_info.max_brightness)
ch->bl = sh_mobile_lcdc_bl_probe(&pdev->dev, ch);
If the return value is not checked then whats the use of return value?
It is checked actually. Look at the places which would dereference
->bl.
quoted
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
-----Original Message-----
From: Dan Carpenter [mailto:error27@gmail.com]
Sent: Monday, March 21, 2011 10:09 AM
To: Janorkar, Mayuresh
Cc: Guennadi Liakhovetski; Magnus Damm; linux-fbdev@vger.kernel.org;
kernel-janitors@vger.kernel.org; Paul Mundt
Subject: Re: [patch] fbdev: sh_mobile_lcdc: checking NULL instead of
IS_ERR()
On Mon, Mar 21, 2011 at 09:47:50AM +0530, Janorkar, Mayuresh wrote:
quoted
quoted
backlight_device_register() returns an ERR_PTR. It doesn't return
NULL.
quoted
The patch is not applying on the master branch of fbdev tree.
I could find another branch: fbdev/shmobile on the tree.
It is a good idea to mention this in the description of the patch.
Sorry, I'm working against linux-next so I wasn't aware.
That's fine :). I saw fbdev in the subject line and To: Paul Mundt so thought this has been developed on fbdev master. But its good idea to mention the base of your patch.
quoted
quoted
Signed-off-by: Dan Carpenter <redacted>
diff --git a/drivers/video/sh_mobile_lcdcfb.c
b/drivers/video/sh_mobile_lcdcfb.c
index bf2629f..a53abe1 100644
*sh_mobile_lcdc_bl_probe(struct device *parent,
bl = backlight_device_register(ch->cfg.bl_info.name, parent, ch,
&sh_mobile_lcdc_bl_ops, NULL);
- if (!bl) {
+ if (IS_ERR(bl)) {
dev_err(parent, "unable to register backlight device\n");
How about printing the error number here?
Ok. That's a good idea.
quoted
quoted
return NULL;
Code is not checking for return value where this function is called.
A code snippet where this function is called:
/* probe the backlight is there is one defined */
if (ch->cfg.bl_info.max_brightness)
ch->bl = sh_mobile_lcdc_bl_probe(&pdev->dev, ch);
If the return value is not checked then whats the use of return value?
It is checked actually. Look at the places which would dereference
->bl.
Yes, got it, it is checked it in start and stop.
quoted
quoted
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev"