Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-19 Thread Roel Kluin
The first loop ends when platform_get_resource() returns NULL. Can it occur that no platform_get_resource() succeeded? I think we should error return if that happens. Could k grow larger than VPIF_DISPLAY_MAX_DEVICES there? Should we err out in that case? In the loop `for (i = 0; i < VPIF_DISPLAY_

Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-19 Thread Roel Kluin
The first loop ends when platform_get_resource() returns NULL. Can it occur that no platform_get_resource() succeeded? I think we should error return if that happens. Could k grow larger than VPIF_DISPLAY_MAX_DEVICES there? Should we err out in that case? In the loop `for (i = 0; i < VPIF_DISPLAY_

Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-19 Thread Roel Kluin
> Ok. You are right! The ch_params[] is a table for keeping the information > about different standards supported. For a given stdid in std_info, the > function matches the stdid with that in the table and get the corresponding > entry. + if (k == VPIF_DISPLAY_MAX_DEVICES) +

RE: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-18 Thread Karicheri, Muralidharan
>>>-      if (!std_info) >>>+      if (!std_info->stdid) >>>               return -1; >>> >> This is a NACK. We shouldn't check for stdid since the function is >supposed >> to update std_info. So just remove >> >> if (!std_info) >>        return -1; > >I don't see how std_info could get updated.

Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-18 Thread roel kluin
>>-      if (!std_info) >>+      if (!std_info->stdid) >>               return -1; >> > This is a NACK. We shouldn't check for stdid since the function is supposed > to update std_info. So just remove > > if (!std_info) >        return -1; I don't see how std_info could get updated. consider the l

RE: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-18 Thread Karicheri, Muralidharan
Roel, Thanks for the patch. >In vpif_get_std_info(): std_info doesn't need the NULL test, it was already >dereferenced anyway. If std_info->stdid is 0 we could early return -1. > >In vpif_probe(): local variable q was only assigned. If we error out with >either last two goto's then j equals VPIF

[PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

2010-02-09 Thread Roel Kluin
In vpif_get_std_info(): std_info doesn't need the NULL test, it was already dereferenced anyway. If std_info->stdid is 0 we could early return -1. In vpif_probe(): local variable q was only assigned. If we error out with either last two goto's then j equals VPIF_DISPLAY_MAX_DEVICES. So after the p