[QUESTION] staging/easycap fix

Greg KH greg at kroah.com
Tue Feb 14 00:06:33 EST 2012


On Tue, Feb 14, 2012 at 01:47:52AM -0300, Ezequiel García wrote:
> Hi,
> 
> I'm try to fix staging/easycap driver. I know it has some bugs
> somewhere (I've seen some panics while using the device) but
> I wanted to improve the code style before debugging it. In the
> meantime, I expect to develop a better understanding of the code.
> However, I want know if I am in the right direction; so I want to know
> if this is ok:
> 
> 1. mainly I am splitting very large functions into smaller parts.  For
> instance, xxx_probe function is +1k lines long, so I'm
> splitting it up to make the code cleaner, more readable. Is this ok?

Yes.

> 2. second, I am fixing some style issues (besides checkpatch), for
> instance "if" syntax:
> 
> -   if (0 == bInterfaceNumber) {
> +   if (bInterfaceNumber == 0) {

You do know why the first style was chosen, right?  That's not saying
your change is incorrect, but odds are, there are bigger things that
need to be fixed up first.

> and ugly comments like:
> 
> -/*---------------------------------------------------------------------------*/
> -/*
> - *  GET PROPERTIES OF PROBED INTERFACE
> - */
> -/*---------------------------------------------------------------------------*/
> +
> +   /*
> +    *  GET PROPERTIES OF PROBED INTERFACE
> +    */
> 
> So, Am I on the right track?

Close, how about:
	/* Get properties of probed interface */
instead?

thanks,

greg k-h



More information about the Kernelnewbies mailing list