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@xxxxxxxxx] Sent: Friday, January 29, 2016 9:55 AM To: Kim, Leo <Leo.Kim@xxxxxxxxx> Cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; linux-wireless <linux-wireless@xxxxxxxxxxxxxxx>; Cho, Tony <Tony.Cho@xxxxxxxxx>; Lee, Glen <Glen.Lee@xxxxxxxxx>; Shin, Austin <Austin.Shin@xxxxxxxxx>; Park, Chris <Chris.Park@xxxxxxxxx>; Abozaeid, Adham <Adham.Abozaeid@xxxxxxxxx>; Ferre, Nicolas <Nicolas.FERRE@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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@xxxxxxxxx Profile: http://www.google.com/profiles/julian.calaby/ _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
