Re: [patch]check NULL pointer

Lukas Czerner
2011-06-14T10:07:18Z

On Tue, 14 Jun 2011, Shaohua Li wrote:

--snip--
> > > Hmm, maybe we just don't use the pointer if it's NULl. it's just print
> > > info anyway.
> > >
> > > Subject: [patch]ext4: check NULL orig_data pointer for mount and remount
> > >
> > > orig_data could be NULL, because the memory allocation of kstrdup()
> > > could fail or data is NULL. Add the NULL check.
> > >
> > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > >
> > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > > index cc5c157..68eba3b 100644
> > > --- a/fs/ext4/super.c
> > > +++ b/fs/ext4/super.c
> > > @@ -3706,7 +3706,7 @@ no_journal:
> > >
> > > ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. "
> > > "Opts: %s%s%s", descr, sbi->s_es->s_mount_opts,
> > > - *sbi->s_es->s_mount_opts ? "; " : "", orig_data);
> > > + *sbi->s_es->s_mount_opts ? "; " : "", orig_data ? : ";");
> >
> > Hi,
> >
> > so you are trying resolve the problem when the allocation fails right ?
> > But what you do is not solving anything, but rather hiding it and it is
> > not different than we had before.
> >
> > So what about this:
> >
> > if (data && !orig_data)
> > return ret;
> how could this work? if data is NULL, orig_data will be NULL, the
> ext4_msg will still use a NULL pointer.
>

Hi,

I am sorry if it was not clear, but as Ted already pointed out printk
can handle NULL pointer, so there is no need to care about it in this
case.

Also it is not a *big* deal not to handle allocation failure in kstrdup,
because there are other allocations in fill_super and remount which will
result in error, however I think that it is better to catch such cases
as early as we can, so it is good to fix this.

Thanks!
-Lukas