Re: [PATCH] I checked with chechpatch.pl the archive indrivers/staging/bcm/CmHost.c , and cleanup the Errors and warnings, I didn'tchange any function, only code style.

[Dan Carpenter]

Hi Tresca,

Welcome.  :)

1) This patch combines way too many things into one patch.  Only do
one logical thing per patch.

2) When you're writing the changelog write a one line summary at the
top, leave a blank line, and then write the changelog.  You didn't
have the summary so the Subject of the email was messed up.

3) No signed-off-by line.

On Mon, Nov 07, 2011 at 08:55:43PM -0300, Tresca Agustin wrote:
> From: Tresca Agustin <atresca@atresca-system.(none)>
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The email address is wrong here.

>
> ---
>  drivers/staging/bcm/CmHost.c | 3794 ++++++++++++++++++++++++-----------------
>  1 files changed, 2221 insertions(+), 1573 deletions(-)
> 
> diff --git a/drivers/staging/bcm/CmHost.c b/drivers/staging/bcm/CmHost.c
> index c0ee95a..8819c52 100644
> --- a/drivers/staging/bcm/CmHost.c
> +++ b/drivers/staging/bcm/CmHost.c
> @@ -4,18 +4,17 @@
>  *	Management.
>  ************************************************************/
>  
> -//#define CONN_MSG
> +/*#define CONN_MSG*/
>  #include "headers.h"
>  
> -typedef enum _E_CLASSIFIER_ACTION
> -{
> -	eInvalidClassifierAction,
> -	eAddClassifier,
> -	eReplaceClassifier,
> -	eDeleteClassifier
> -}E_CLASSIFIER_ACTION;
> +enum _E_CLASSIFIER_ACTION {
> +	eInvalidClsfrAction,
> +	eAddClsfr,
> +	eRplcClsfr,
> +	eDelClsfr
> +}; /*_E_CLASSIFIER_ACTION*/

These names are even worse than the original.

>  
> -static ULONG GetNextTargetBufferLocation(PMINI_ADAPTER Adapter,B_UINT16 tid);
> +static ULONG GetNextTargetBufferLocation(PMINI_ADAPTER Adapter, B_UINT16 tid);
>  
>  /************************************************************
>  * Function	  -	SearchSfid
> @@ -24,18 +23,18 @@ static ULONG GetNextTargetBufferLocation(PMINI_ADAPTER Adapter,B_UINT16 tid);
>  *				specified SFID as input parameter.
>  *
>  * Parameters  -	Adapter: Pointer to the Adapter structure
> -*			   	uiSfid : Given SFID for matching
> +*				uiSfid : Given SFID for matching
>  *
>  * Returns	  - Queue index for this SFID(If matched)
>  				Else Invalid Queue Index(If Not matched)
>  ************************************************************/
> -INT SearchSfid(PMINI_ADAPTER Adapter,UINT uiSfid)
> +INT SearchSfid(PMINI_ADAPTER Adapter, UINT uiSfid)
>  {
> -	INT 	iIndex=0;
> -	for(iIndex=(NO_OF_QUEUES-1); iIndex>=0; iIndex--)
> -		if(Adapter->PackInfo[iIndex].ulSFID==uiSfid)
> +	INT iIndex = 0;

blank line goes here.
INT should be changed to "int"
No need to initialize iIndex.

> +	for (iIndex = (NO_OF_QUEUES - 1); iIndex >= 0; iIndex--)
> +		if (Adapter->PackInfo[iIndex].ulSFID == uiSfid)
>  			return iIndex;

But curly braces around multi-line indent blocks for style reasons
even though it's not needed for semantic reasons.

> -	return NO_OF_QUEUES+1;
> +	return NO_OF_QUEUES + 1;

Anyway, this whole thing needs to be broken up into smaller logical
patches and resent.  Like maybe change fix add spaces around the
operators in one patch or something like that.  Fix one class of
checkpatch.pl warning at a time.

Don't worry that your first patch was not accepted.  Everyone's first
patch gets rejected.  :P

regards,
dan carpenter

Attachment: signature.asc
Description: Digital signature