| Message ID | 20171108212847.17952-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | 4b29975fd0f25256d8e643076a2c4c5f9de00f8f |
| Delegated to: | Heiko Schocher |
| Headers | show |
| Series | [U-Boot,1/1] ubifs: no NULL check needed before free | expand |
Hello Heinrich, Am 08.11.2017 um 22:28 schrieb Heinrich Schuchardt: > kfree() calls free. > free() checks if the parameter is NULL. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > fs/ubifs/ubifs.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) applied to u-boot-ubi master Thanks! bye, Heiko
Hi Heinrich and Heiko,
do you see anything strange in this code (it was more obvious before
the patch but it can still be spotted)? I should say that it's too
late to check for "file != NULL" after the "file->private_data"
dereference. Maybe it should look like this:
if (file)
kfree(file->private_data);
free(file);
shouldn't it?
Cheers, Alex.
On 11/21/2017 10:20 AM, Alex Sadovsky wrote: > Hi Heinrich and Heiko, > > do you see anything strange in this code (it was more obvious before > the patch but it can still be spotted)? I should say that it's too > late to check for "file != NULL" after the "file->private_data" > dereference. Maybe it should look like this: > if (file) > kfree(file->private_data); > free(file); > shouldn't it? Thanks for spotting this. Probably we should fix it here: if (!file || !dentry || !dir) { printf("%s: Error, no memory for malloc!\n", __func__); err = -ENOMEM; goto out; } Why should we first print printf("%s: Error, no memory for malloc!\n", __func__); and then dbg_gen("cannot find next direntry, error %d", err); Instead we should immediately return. Regards Heinrich
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 8f1c9d167d..4465523d5f 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -462,14 +462,10 @@ out: dbg_gen("cannot find next direntry, error %d", err); out_free: - if (file->private_data) - kfree(file->private_data); - if (file) - free(file); - if (dentry) - free(dentry); - if (dir) - free(dir); + kfree(file->private_data); + free(file); + free(dentry); + free(dir); return ret; }
kfree() calls free. free() checks if the parameter is NULL. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/ubifs/ubifs.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)