2009-12-28 15:55:12

by Martin Jansa

Subject: [PATCH] pxafb: fix regression from d2a34c13e7ccec5d06eafd60e6f80ea531b34668

fbi->dev->platform_data is void * and gcc does not allow to access
!fbi->dev->platform_data->smart_update without cast platform_data to
(struct pxafb_mach_info) or assignment as used in this patch.

Signed-off-by: Martin Jansa <[email protected]>
---
drivers/video/pxafb.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
index 415858b..241468c 100644
--- a/drivers/video/pxafb.c
+++ b/drivers/video/pxafb.c
@@ -1223,12 +1223,17 @@ static int pxafb_smart_thread(void *arg)
struct pxafb_info *fbi = arg;
struct pxafb_mach_info *inf;

- if (!fbi || !fbi->dev->platform_data->smart_update) {
+ if (!fbi) {
pr_err("%s: not properly initialized, thread terminated\n",
__func__);
return -EINVAL;
}
inf = fbi->dev->platform_data;
+ if (!inf || !inf->smart_update) {
+ pr_err("%s: not properly initialized, thread terminated\n",
+ __func__);
+ return -EINVAL;
+ }

pr_debug("%s(): task starting\n", __func__);

--
1.6.6

2010-01-05 23:00:13

by Andrew Morton

Subject: Re: [PATCH] pxafb: fix regression from d2a34c13e7ccec5d06eafd60e6f80ea531b34668

On Mon, 28 Dec 2009 16:54:43 +0100
Martin Jansa <[email protected]> wrote:

> fbi->dev->platform_data is void * and gcc does not allow to access
> !fbi->dev->platform_data->smart_update without cast platform_data to
> (struct pxafb_mach_info) or assignment as used in this patch.
>
> Signed-off-by: Martin Jansa <[email protected]>
> ---
> drivers/video/pxafb.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
> index 415858b..241468c 100644
> --- a/drivers/video/pxafb.c
> +++ b/drivers/video/pxafb.c
> @@ -1223,12 +1223,17 @@ static int pxafb_smart_thread(void *arg)
> struct pxafb_info *fbi = arg;
> struct pxafb_mach_info *inf;
>
> - if (!fbi || !fbi->dev->platform_data->smart_update) {
> + if (!fbi) {
> pr_err("%s: not properly initialized, thread terminated\n",
> __func__);
> return -EINVAL;
> }
> inf = fbi->dev->platform_data;
> + if (!inf || !inf->smart_update) {
> + pr_err("%s: not properly initialized, thread terminated\n",
> + __func__);
> + return -EINVAL;
> + }
>
> pr_debug("%s(): task starting\n", __func__);
>

This got fixed differently in Eric's tree. The patch is still in
linux-next, not mainline:

: commit da2c3f0ead336c04b4d1ad36ff42d8d264f44f65
: Author: Eric Miao <[email protected]>
: AuthorDate: Sat Dec 26 16:25:18 2009 +0800
: Commit: Eric Miao <[email protected]>
: CommitDate: Tue Dec 29 14:11:27 2009 +0800
:
: [ARM] pxafb: fix building issue of incorrect reference
:
: Commit "d2a34c1 drivers/video: Move dereference after NULL test" introduced
: a build error of "fbi->dev->platform_data->smart_update" being unknown type
: to the compiler, fix this by removing the unnecessary test of 'fbi'.
:
: Cc: Julia Lawall <[email protected]>
: Signed-off-by: Eric Miao <[email protected]>
:
: diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
: index 415858b..825b665 100644
: --- a/drivers/video/pxafb.c
: +++ b/drivers/video/pxafb.c
: @@ -1221,9 +1221,9 @@ static void setup_smart_timing(struct pxafb_info *fbi,
: static int pxafb_smart_thread(void *arg)
: {
: struct pxafb_info *fbi = arg;
: - struct pxafb_mach_info *inf;
: + struct pxafb_mach_info *inf = fbi->dev->platform_data;
:
: - if (!fbi || !fbi->dev->platform_data->smart_update) {
: + if (!inf->smart_update) {
: pr_err("%s: not properly initialized, thread terminated\n",
: __func__);
: return -EINVAL;
:

That patch fixes the build, but it also eliminates the test for
fbi==NULL. The changelog doesn't explain that change and I wonder if
it was intentional and correct?

2010-01-06 00:19:08

by Eric Miao

Subject: Re: [PATCH] pxafb: fix regression from d2a34c13e7ccec5d06eafd60e6f80ea531b34668

>
> That patch fixes the build, but it also eliminates the test for
> fbi==NULL.  The changelog doesn't explain that change and I wonder if
> it was intentional and correct?
>
>

fbi has been well tested in pxafb_probe() and then passed to pxafb_smart_init()
and to pxafb_smart_update(), it's just unnecessary to test it again and again.