Re: [PATCH v2 01/11] staging: wilc1000: replace WILC_ERRORREPORT with return

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 15, 2015 at 01:07:06PM +0300, Mike Rapoport wrote:
> In cases where WILC_ERRORREPORT does not require cleanup actions, but
> causes immediate return from the function it can be replaced with return
> statement.
> 
> Signed-off-by: Mike Rapoport <mike.rapoport@xxxxxxxxx>
> ---
>  drivers/staging/wilc1000/coreconfigurator.c       |   3 +-
>  drivers/staging/wilc1000/host_interface.c         | 173 +++++++---------------
>  drivers/staging/wilc1000/wilc_msgqueue.c          |  29 ++--
>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c |   2 +-
>  4 files changed, 66 insertions(+), 141 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> index 544c12d..566fb59 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.c
> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> @@ -594,7 +594,7 @@ s32 ParseNetworkInfo(u8 *pu8MsgBuffer, tstrNetworkInfo **ppstrNetworkInfo)
>  	/* Check whether the received message type is 'N' */
>  	if ('N' != u8MsgType) {
>  		PRINT_ER("Received Message format incorrect.\n");
> -		WILC_ERRORREPORT(s32Error, WILC_FAIL);
> +		return WILC_FAIL;
>  	}
>  
>  	/* Extract message ID */
> @@ -690,7 +690,6 @@ s32 ParseNetworkInfo(u8 *pu8MsgBuffer, tstrNetworkInfo **ppstrNetworkInfo)
>  
>  	*ppstrNetworkInfo = pstrNetworkInfo;
>  
> -ERRORHANDLER:
>  	return s32Error;
>  }
>  
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 59a1a9d..d75b38d 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1318,13 +1318,13 @@ static s32 Handle_Scan(tstrWILC_WFIDrv *drvHandler, tstrHostIFscanAttr *pstrHost
>  	if ((pstrWFIDrv->enuHostIFstate >= HOST_IF_SCANNING) && (pstrWFIDrv->enuHostIFstate < HOST_IF_CONNECTED)) {
>  		/* here we either in HOST_IF_SCANNING, HOST_IF_WAITING_CONN_REQ or HOST_IF_WAITING_CONN_RESP */
>  		PRINT_D(GENERIC_DBG, "Don't scan we are already in [%d] state\n", pstrWFIDrv->enuHostIFstate);
> -		WILC_ERRORREPORT(s32Error, WILC_BUSY);
> +		return WILC_BUSY;
>  	}
>  
>  	#ifdef DISABLE_PWRSAVE_AND_SCAN_DURING_IP
>  	if (g_obtainingIP || connecting) {
>  		PRINT_D(GENERIC_DBG, "[handle_scan]: Don't do obss scan until IP adresss is obtained\n");
> -		WILC_ERRORREPORT(s32Error, WILC_BUSY);
> +		return WILC_BUSY;
>  	}
>  	#endif
>  
> @@ -2298,7 +2298,7 @@ static s32 Handle_RcvdNtwrkInfo(tstrWILC_WFIDrv *drvHandler, tstrRcvdNetworkInfo
>  		ParseNetworkInfo(pstrRcvdNetworkInfo->pu8Buffer, &pstrNetworkInfo);
>  		if ((pstrNetworkInfo == NULL)
>  		    || (pstrWFIDrv->strWILC_UsrScanReq.pfUserScanResult == NULL)) {
> -			WILC_ERRORREPORT(s32Error, WILC_INVALID_ARGUMENT);
> +			return WILC_INVALID_ARGUMENT;

Are you sure about that?  Because:

>  		}
>  
>  		/* check whether this network is discovered before */
> @@ -2365,12 +2365,6 @@ static s32 Handle_RcvdNtwrkInfo(tstrWILC_WFIDrv *drvHandler, tstrRcvdNetworkInfo
>  		}
>  	}
>  
> -
> -	WILC_CATCH(s32Error)
> -	{
> -
> -	}
> -
>  done:
>  	/* Deallocate pstrRcvdNetworkInfo->pu8Buffer which was prevoisuly allocated by the sending thread */
>  	if (pstrRcvdNetworkInfo->pu8Buffer != NULL) {

You do a bunch of work here, if that error occurs.

You just changed the logic of this function :(

Please be much more careful when cleaning stuff up like error paths.  I
suggest you redo this whole series, possibly looking at the other set of
patches that was recently posted to do much this same thing, to verify
that you really are not breaking anything.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux