From mboxrd@z Thu Jan  1 00:00:00 1970
From: greg@kroah.com (Greg KH)
Date: Mon, 13 Feb 2012 21:06:33 -0800
Subject: [QUESTION] staging/easycap fix
In-Reply-To: <CALF0-+Ue+MBB531WgsQNjCBdkPe=tSLB1fAcyfX-Wa_o_rZAGg@mail.gmail.com>
References: <CALF0-+UdbqOnQ6ZT0bPiMVCTFWwMQss6XrFqNL6xsugRPVX_qw@mail.gmail.com>
	<CALF0-+Ue+MBB531WgsQNjCBdkPe=tSLB1fAcyfX-Wa_o_rZAGg@mail.gmail.com>
Message-ID: <20120214050633.GA30972@kroah.com>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

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