[PATCH 08/12] staging: wilc1000: rename variable s32Error

Kim, Leo Leo.Kim at atmel.com
Fri Jan 29 02:55:12 UTC 2016


Dear Julian

Thank you in your advises.
This patch subject is 'rename variables'.
This time, I hope the patch is applied.

I will be your positive advises change to next patch.

 Thanks, BR
 Leo

-----Original Message-----
From: Julian Calaby [mailto:julian.calaby at gmail.com] 
Sent: Friday, January 29, 2016 9:55 AM
To: Kim, Leo <Leo.Kim at atmel.com>
Cc: Greg KH <gregkh at linuxfoundation.org>; devel at driverdev.osuosl.org; linux-wireless <linux-wireless at vger.kernel.org>; Cho, Tony <Tony.Cho at atmel.com>; Lee, Glen <Glen.Lee at atmel.com>; Shin, Austin <Austin.Shin at atmel.com>; Park, Chris <Chris.Park at atmel.com>; Abozaeid, Adham <Adham.Abozaeid at atmel.com>; Ferre, Nicolas <Nicolas.FERRE at atmel.com>
Subject: Re: [PATCH 08/12] staging: wilc1000: rename variable s32Error

Hi Leo,

On Thu, Jan 28, 2016 at 6:13 PM, Leo Kim <leo.kim at atmel.com> wrote:
> This patch renames variable s32Error to result to avoid CamelCase 
> naming convention.
> Also, remove the unused variable s32Error and replace with direct return.
>
> Signed-off-by: Leo Kim <leo.kim at atmel.com>
> ---
>  drivers/staging/wilc1000/coreconfigurator.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.c 
> b/drivers/staging/wilc1000/coreconfigurator.c
> index 2a4e324..88e5661 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.c
> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> @@ -356,30 +356,29 @@ s32 wilc_parse_network_info(u8 *msg_buffer, 
> tstrNetworkInfo **ret_network_info)
>
>  s32 wilc_dealloc_network_info(tstrNetworkInfo *pstrNetworkInfo)  {
> -       s32 s32Error = 0;
> +       s32 result = 0;
>
>         if (pstrNetworkInfo) {
>                 if (pstrNetworkInfo->pu8IEs) {
>                         kfree(pstrNetworkInfo->pu8IEs);
>                         pstrNetworkInfo->pu8IEs = NULL;

I'm not sure it's necessary to set this to NULL here.

>                 } else {
> -                       s32Error = -EFAULT;
> +                       result = -EFAULT;
>                 }
>
>                 kfree(pstrNetworkInfo);
>                 pstrNetworkInfo = NULL;

Or this to NULL here.

Also, is the return value from this function actually checked? kfree()
can have NULL passed to it.

If the return value isn't checked, this entire function could be simplified to:

- - - - -

if (pstrNetworkInfo) {
    kfree(pstrNetworkInfo->pu8IEs);
    kfree(pstrNetworkInfo);
}

return 0;

- - - - -

Also, when tstrNetworkInfo structs are allocated, wouldn't the
allocating function fail if ->pu8IEs is NULL?

>         } else {
> -               s32Error = -EFAULT;
> +               result = -EFAULT;
>         }
>
> -       return s32Error;
> +       return result;
>  }
>
>  s32 wilc_parse_assoc_resp_info(u8 *pu8Buffer, u32 u32BufferLen,
>                                tstrConnectRespInfo **ppstrConnectRespInfo)
>  {
> -       s32 s32Error = 0;
>         tstrConnectRespInfo *pstrConnectRespInfo = NULL;
>         u16 u16AssocRespLen = 0;
>         u8 *pu8IEs = NULL;
> @@ -408,27 +407,27 @@ s32 wilc_parse_assoc_resp_info(u8 *pu8Buffer, u32 u32BufferLen,
>
>         *ppstrConnectRespInfo = pstrConnectRespInfo;
>
> -       return s32Error;
> +       return 0;
>  }
>
>  s32 wilc_dealloc_assoc_resp_info(tstrConnectRespInfo *pstrConnectRespInfo)
>  {
> -       s32 s32Error = 0;
> +       s32 result = 0;

All the comments above apply to this function as well.

>
>         if (pstrConnectRespInfo) {
>                 if (pstrConnectRespInfo->pu8RespIEs) {
>                         kfree(pstrConnectRespInfo->pu8RespIEs);
>                         pstrConnectRespInfo->pu8RespIEs = NULL;
>                 } else {
> -                       s32Error = -EFAULT;
> +                       result = -EFAULT;
>                 }
>
>                 kfree(pstrConnectRespInfo);
>                 pstrConnectRespInfo = NULL;
>
>         } else {
> -               s32Error = -EFAULT;
> +               result = -EFAULT;
>         }
>
> -       return s32Error;
> +       return result;
>  }

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


More information about the devel mailing list