On Tue, Feb 14, 2012 at 07:01:25PM -0300, Ezequiel Garc?a wrote:

Hi Greg,

quoted

It's only "recommended" if you have a compiler that doesn't check for
such foolish things.

Understood. Thanks both.

Another question (hope you don't mind me asking so much):

I noticed easycap does lots of (redundant?) checks as:

pvideo_device = video_devdata(file);
if (!pvideo_device) {
  return -EFAULT;
}

Is this bad, good or doesn't matter?
(There are more examples where the check is clearly not needed but
this one makes me doubt a bit)

Depends on what video_devdata() does, and how it could ever be NULL.

Maybe I'm focusing on small issues (as you already pointed out),
do you think this kind of patches would be accepted?

Of course, I would submit separate patches, one for each change:
1. split probe function
2. remove redundant checks
3. clean comment style
... and so on.

That sounds great.

Also, I'm reading driver cx231xx as it's also an usb video capture.
Do you think it's a good code reference? Can you give me further reference?

I don't know, ask the linux-media developers on their list for a good
reference driver to follow, they would know best.

thanks,

greg k-h