fbdev: sh_mobile_lcdc: checking NULL instead of IS_ERR()

PATCH Superseded Dan Carpenter 2 participants 4 messages Mar 19, 2011 – Mar 21, 2011

Subsystems (2)

Kernel janitors Framebuffer layer

lore

Share & tools

lore

Dan Carpenter

author

2011-03-19 04:36 UTC #0 lore raw

PATCH 1/1 fbdev: sh_mobile_lcdc: checking NULL instead of IS_ERR()

backlight_device_register() returns an ERR_PTR. It doesn't return NULL.

Signed-off-by Dan Carpenter <[email protected]>

drivers/video/sh_mobile_lcdcfb.c +1 -1

index bf2629f..a53abe1 100644
@@ -1088,7 +1088,7 @@ static struct backlight_device *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");
 		return NULL;
 	}

Janorkar, Mayuresh

2011-03-21 04:29 UTC #1 lore raw

Quoted 11 lines

-----Original Message-----
From: [email protected] [mailto:linux-fbdev- [email protected]] On Behalf Of Dan Carpenter
Sent: Saturday, March 19, 2011 10:07 AM
To: Paul Mundt
Cc: Guennadi Liakhovetski; Magnus Damm; [email protected]; [email protected]
Subject: [patch] fbdev: sh_mobile_lcdc: checking NULL instead of IS_ERR()

Quoted 1 line

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 9 lines

Signed-off-by Dan Carpenter <[email protected]>
drivers/video/sh_mobile_lcdcfb.c +0 -0
index bf2629f..a53abe1 100644

drivers/video/sh_mobile_lcdcfb.c static struct backlight_device · line 1093

Quoted 7 lines

drivers/video/sh_mobile_lcdcfb.c +1 -1

@@ -1088,7 +1088,7 @@ static struct backlight_device
 *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?

drivers/video/sh_mobile_lcdcfb.c static struct backlight_device *sh_mobile_lcdc_bl_probe(struct device *parent, · line 1093

Quoted 1 line

drivers/video/sh_mobile_lcdcfb.c +0 -0
 		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?

Quoted 5 lines

}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Dan Carpenter

author

2011-03-21 04:39 UTC #2 lore raw

Re: [patch] fbdev: sh_mobile_lcdc: checking NULL instead of

In reply to Dan Carpenter

Quoted 6 lines · Mayuresh

Quoted 1 line
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 11 lines

Quoted 11 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/video/sh_mobile_lcdcfb.c +0 -0
index bf2629f..a53abe1 100644
@@ -1088,7 +1088,7 @@ static struct backlight_device
 *sh_mobile_lcdc_bl_probe(struct device *parent,
 

Quoted 9 lines

drivers/video/sh_mobile_lcdcfb.c static struct backlight_device *sh_mobile_lcdc_bl_probe(struct device *parent, · line 1092
Quoted 5 lines
drivers/video/sh_mobile_lcdcfb.c +1 -1
 	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 11 lines

drivers/video/sh_mobile_lcdcfb.c static struct backlight_device *sh_mobile_lcdc_bl_probe(struct device *parent, · line 1093
Quoted 1 line
drivers/video/sh_mobile_lcdcfb.c +0 -0
 		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 5 lines

Quoted 5 lines
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Janorkar, Mayuresh

2011-03-21 05:43 UTC #3 lore raw

In reply to Dan Carpenter

Quoted 13 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Monday, March 21, 2011 10:09 AM
To: Janorkar, Mayuresh
Cc: Guennadi Liakhovetski; Magnus Damm; [email protected]; [email protected]; Paul Mundt
Subject: Re: [patch] fbdev: sh_mobile_lcdc: checking NULL instead of IS_ERR()
Quoted 1 line · Mayuresh
Quoted 1 line
backlight_device_register() returns an ERR_PTR. It doesn't return

Quoted 8 lines

NULL.
Quoted 5 lines
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 33 lines

Quoted 12 lines
Quoted 12 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/video/sh_mobile_lcdcfb.c +0 -0
index bf2629f..a53abe1 100644
@@ -1088,7 +1088,7 @@ static struct backlight_device
 *sh_mobile_lcdc_bl_probe(struct device *parent,
 
Quoted 9 lines
drivers/video/sh_mobile_lcdcfb.c static struct backlight_device *sh_mobile_lcdc_bl_probe(struct device *parent, · line 1092
Quoted 5 lines
drivers/video/sh_mobile_lcdcfb.c +1 -1
 	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 9 lines
drivers/video/sh_mobile_lcdcfb.c static struct backlight_device *sh_mobile_lcdc_bl_probe(struct device *parent, · line 1093
Quoted 1 line
drivers/video/sh_mobile_lcdcfb.c +0 -0
 		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);

Quoted 5 lines

Quoted 2 lines
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 7 lines

Quoted 4 lines
Quoted 4 lines
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev"
in
Quoted 2 lines
Quoted 2 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html