[patch] hid: picolcd: testing the wrong variable

Subsystems: hid core layer, picolcd hid driver, the rest

STALE5911d

4 messages, 3 authors, 2010-08-06 · open the first message on its own page

[patch] hid: picolcd: testing the wrong variable

From: Dan Carpenter <hidden>
Date: 2010-08-06 06:29:14

"ref_cnt" is a point to the reference count and it's non-null.  We really
want to test the reference count itself.

Signed-off-by: Dan Carpenter <redacted>

diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 346f0e3..c0bdeba 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,7 +547,7 @@ static void picolcd_fb_destroy(struct fb_info *info)
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
 		framebuffer_release(info);

Re: [patch] hid: picolcd: testing the wrong variable

From: Bruno Prémont <bonbons@linux-vserver.org>
Date: 2010-08-06 08:15:33

Thanks for spotting!

But while fixing this one also fix the ordering (which was never
reached due to this wrong check) a few lines later.

Patch (combined) at the end.

Thanks,
Bruno


On Fri, 6 Aug 2010 08:28:49 Dan Carpenter [off-list ref] wrote:

quoted hunk

"ref_cnt" is a point to the reference count and it's non-null.  We really
want to test the reference count itself.

Signed-off-by: Dan Carpenter <redacted>

diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 346f0e3..c0bdeba 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,7 +547,7 @@ static void picolcd_fb_destroy(struct fb_info *info)
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
 		framebuffer_release(info);

--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,11 +547,11 @@ static void picolcd_fb_destroy(struct fb
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
-		framebuffer_release(info);
 		vfree((u8 *)info->fix.smem_start);
+		framebuffer_release(info);
 	}
 }

Re: [patch] hid: picolcd: testing the wrong variable

From: Jiri Kosina <hidden>
Date: 2010-08-06 12:53:40

On Fri, 6 Aug 2010, Dan Carpenter wrote:

quoted hunk

"ref_cnt" is a point to the reference count and it's non-null.  We really
want to test the reference count itself.

Signed-off-by: Dan Carpenter <redacted>

diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 346f0e3..c0bdeba 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,7 +547,7 @@ static void picolcd_fb_destroy(struct fb_info *info)
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
 		framebuffer_release(info);

Thanks a lot for spotting this. Applied.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [patch] hid: picolcd: testing the wrong variable

From: Jiri Kosina <hidden>
Date: 2010-08-06 12:54:24

On Fri, 6 Aug 2010, Bruno Prémont wrote:

quoted hunk

Thanks for spotting!

But while fixing this one also fix the ordering (which was never
reached due to this wrong check) a few lines later.

Patch (combined) at the end.

Thanks,
Bruno


On Fri, 6 Aug 2010 08:28:49 Dan Carpenter [off-list ref] wrote:

quoted

"ref_cnt" is a point to the reference count and it's non-null.  We really
want to test the reference count itself.

Signed-off-by: Dan Carpenter <redacted>

diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 346f0e3..c0bdeba 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,7 +547,7 @@ static void picolcd_fb_destroy(struct fb_info *info)
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
 		framebuffer_release(info);

--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c

@@ -547,11 +547,11 @@ static void picolcd_fb_destroy(struct fb
 	ref_cnt--;
 	mutex_lock(&info->lock);
 	(*ref_cnt)--;
-	may_release = !ref_cnt;
+	may_release = !*ref_cnt;
 	mutex_unlock(&info->lock);
 	if (may_release) {
-		framebuffer_release(info);
 		vfree((u8 *)info->fix.smem_start);
+		framebuffer_release(info);
 	}
 }

I have applied the patch fixing the ordering, along with adding

	Signed-off-by: Bruno Prémont [off-list ref]

to it. Please shoud if that is not proper.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help