| Message ID | 20241219110027.1440876-5-shikemeng@huaweicloud.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix and cleanups to ext4 namei.c | expand |
On Thu, Dec 19, 2024 at 07:00:25PM +0800, Kemeng Shi wrote: > The "offset" is always non-NULL, remove unneeded NULL check of "offset". > > Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> Hi Kemeng, I know the current callers don't pass NULL but I think we should still keep the check around just in case, to avoid NULL dereferences in future. I don't think there's any harm in keeping it Regards, ojaswin > --- > fs/ext4/namei.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 33670cebdedc..07a1bb570deb 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -434,8 +434,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode, > } else > return NULL; > > - if (offset) > - *offset = count_offset; > + *offset = count_offset; > return (struct dx_countlimit *)(((void *)dirent) + count_offset); > } > > -- > 2.30.0 >
On 2024/12/19 19:00, Kemeng Shi wrote: > The "offset" is always non-NULL, remove unneeded NULL check of "offset". > > Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> I agree with Ojaswin, the check strengthens the function, and I'd suggest that we'd better to keep it for now. Thanks, Yi. > --- > fs/ext4/namei.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 33670cebdedc..07a1bb570deb 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -434,8 +434,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode, > } else > return NULL; > > - if (offset) > - *offset = count_offset; > + *offset = count_offset; > return (struct dx_countlimit *)(((void *)dirent) + count_offset); > } >
on 12/20/2024 8:51 PM, Ojaswin Mujoo wrote: > On Thu, Dec 19, 2024 at 07:00:25PM +0800, Kemeng Shi wrote: >> The "offset" is always non-NULL, remove unneeded NULL check of "offset". >> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> > > Hi Kemeng, > > I know the current callers don't pass NULL but I think we should still > keep the check around just in case, to avoid NULL dereferences in > future. I don't think there's any harm in keeping it > Sure, no insistant on this and will drop this in next version. Thanks, Kemeng > Regards, > ojaswin > >> --- >> fs/ext4/namei.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c >> index 33670cebdedc..07a1bb570deb 100644 >> --- a/fs/ext4/namei.c >> +++ b/fs/ext4/namei.c >> @@ -434,8 +434,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode, >> } else >> return NULL; >> >> - if (offset) >> - *offset = count_offset; >> + *offset = count_offset; >> return (struct dx_countlimit *)(((void *)dirent) + count_offset); >> } >> >> -- >> 2.30.0 >> >
on 12/21/2024 3:44 PM, Zhang Yi wrote: > On 2024/12/19 19:00, Kemeng Shi wrote: >> The "offset" is always non-NULL, remove unneeded NULL check of "offset". >> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> > > I agree with Ojaswin, the check strengthens the function, and I'd > suggest that we'd better to keep it for now. Sure, will drop this in next version. Thanks, Kemeng > > Thanks, > Yi. > >> --- >> fs/ext4/namei.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c >> index 33670cebdedc..07a1bb570deb 100644 >> --- a/fs/ext4/namei.c >> +++ b/fs/ext4/namei.c >> @@ -434,8 +434,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode, >> } else >> return NULL; >> >> - if (offset) >> - *offset = count_offset; >> + *offset = count_offset; >> return (struct dx_countlimit *)(((void *)dirent) + count_offset); >> } >> >
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 33670cebdedc..07a1bb570deb 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -434,8 +434,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode, } else return NULL; - if (offset) - *offset = count_offset; + *offset = count_offset; return (struct dx_countlimit *)(((void *)dirent) + count_offset); }
The "offset" is always non-NULL, remove unneeded NULL check of "offset". Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> --- fs/ext4/namei.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)