Re: [PATCH] [media] gspca: replaced static allocation by video_device_alloc/video_device_release

[Ezequiel <elezegarcia@gmail.com>]

On Sat, Nov 19, 2011 at 08:20:22PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/19/2011 07:50 PM, Ezequiel wrote:
> > Pushed video_device initialization into a separate function.
> > Replaced static allocation of struct video_device by
> > video_device_alloc/video_device_release usage.
> 
> NACK! I see a video_device_release call here, but not a
> video_device_alloc, also you're messing with quite sensitive code
> here (because a usb device can be unplugged at any time, including
> when the /dev/video node is open by a process), and changing it
> from static to dynamic allocation my have more consequences
> then you see at first (I did not analyze all the code paths
> for the proposed change, since the last time I audited them for
> the current static allocation of the videodevice struct code took
> me hours).
> 
> Also static allocation (as part of the driver struct) in general is
> better then dynamic as it needs less code and helps avoiding memory
> fragmentation.
> 
> All in all I cannot help but feel that you're diving into a piece
> of code with some drive by shooting style patch without knowing
> the code in question at all, please don't do that!
> 
> Regards,
> 
> Hans
> 

Hi Hans,

Sorry, really dont know what happened, 
I sent an incomplete patch version.
(some vim yank-key error).

I understand your observations about static vs dynamic, 
but please could you review the right patch.

Thanks,
Ezequiel.