Thank you for the review Joe. I'll have that in mind in the next patch. On 3/9/19, Joe Perches <joe@xxxxxxxxxxx> wrote: > On Sat, 2019-03-09 at 15:30 -0300, Guilherme Tadashi Maeoka wrote: >> Fix an assignment in if condition. >> >> Signed-off-by: Guilherme Tadashi Maeoka <gui.maeoka@xxxxxxxxx> >> --- >> drivers/staging/rtl8723bs/os_dep/osdep_service.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c >> b/drivers/staging/rtl8723bs/os_dep/osdep_service.c >> index 73b87da15eb2..c7fdcc6bbae3 100644 >> --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c >> +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c >> @@ -162,7 +162,9 @@ static int retriveFromFile(char *path, u8 *buf, u32 >> sz) >> struct file *fp; >> >> if (path && buf) { >> - if (0 == (ret =openFile(&fp, path, O_RDONLY, 0))) { >> + ret = openFile(&fp, path, O_RDONLY, 0); >> + >> + if (ret == 0) { >> DBG_871X("%s openFile path:%s fp =%p\n", __func__, path , fp); >> >> oldfs = get_fs(); set_fs(KERNEL_DS); > > More common kernel style would be to rewrite the function > (and to fix the typo of retrieve) to something like: > > static int retrieveFromFile(char *path, u8 *buf, u32 sz) > { > int ret; > mm_segment_t oldfs; > struct file *fp; > > if (!path || !buf) { > DBG_871X("%s NULL pointer\n", __func__); > return -EINVAL; > } > > ret = openFile(&fp, path, O_RDONLY, 0); > if (ret) { > DBG_871X("%s openFile path:%s Fail, ret:%d\n", > __func__, path, ret); > return ret; > } > > DBG_871X("%s openFile path:%s fp =%p\n", __func__, path , fp); > > oldfs = get_fs(); > set_fs(KERNEL_DS); > ret = readFile(fp, buf, sz); > set_fs(oldfs); > closeFile(fp); > > DBG_871X("%s readFile, ret:%d\n", __func__, ret); > > return ret; > } > > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel