On Wed, Nov 22, 2017 at 09:09:36AM +0100, Wolfgang Denk wrote: > Dear Ladislav, > > In message <20171121212222.ryicwv6tyh5rye2e@lenoch> you wrote: > > > > > > > > diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c > > > > index 4465523d5f..313dee0579 100644 > > > > --- a/fs/ubifs/ubifs.c > > > > +++ b/fs/ubifs/ubifs.c > > > > @@ -403,8 +403,7 @@ static int ubifs_finddir(struct super_block *sb, > > > > char *dirname, > > > > dir = kzalloc(sizeof(struct inode), 0); > > > > if (!file || !dentry || !dir) { > > > > printf("%s: Error, no memory for malloc!\n", __func__); > > > > - err = -ENOMEM; > > > > - goto out; > > > > + goto out_nomem; > > > > } > ... > > > Should you not keep the "err = -ENOMEM;" setting? Otherwise there > > > is no indivcation that an error happened. > > > > It is not obvious from the patch, but value of err is later discarded. > > It serves sole purpose of printing debug notice. > > So apparently we have a number of places in U-Boot where fatal > errors (running out of memory) are just ignored and we continue as > if nothing happened?
While I have to admit this code is not an example of clean coding, it prints notice when trying to manipulate with file. fs/ubifs/ubifs.c as whole needs to be revisited, above patch just caused shit hitting the fan. > THis is short-sighted at best. One day Pump Six will fail. > > This is giving me the creepes. I was just pointing to the fact, that above mentioned patch does not make it any worse. Btw, initial commit is even more amazing: + if (file) + free(file); + if (dentry) + free(dentry); + if (dir) + free(dir); + + if (file->private_data) + kfree(file->private_data); + file->private_data = NULL; + file->f_pos = 2; + return 0; .oO(I guess it is safe not pointing where above snippet is comming from, but will review whole file as I'm going to use it for new board I have to suport) Best regards, ladis _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot