| Message ID | 20220524204040.954138-1-preichl@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mkfs: Fix memory leak | expand |
On Tue, May 24, 2022 at 10:40:40PM +0200, Pavel Reichl wrote: > 'value' is allocated by strup() in getstr(). It Nit: strdup, not strup. > needs to be freed as we do not keep any permanent > reference to it. > > Signed-off-by: Pavel Reichl <preichl@redhat.com> With that fixed, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > mkfs/xfs_mkfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 01d2e8ca..a37d6848 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -1714,6 +1714,7 @@ naming_opts_parser( > } else { > cli->sb_feat.dir_version = getnum(value, opts, subopt); > } > + free((char *)value); > break; > case N_FTYPE: > cli->sb_feat.dirftype = getnum(value, opts, subopt); > -- > 2.36.1 >
On Tue, May 24, 2022 at 02:06:44PM -0700, Darrick J. Wong wrote: > On Tue, May 24, 2022 at 10:40:40PM +0200, Pavel Reichl wrote: > > 'value' is allocated by strup() in getstr(). It > > Nit: strdup, not strup. > > > needs to be freed as we do not keep any permanent > > reference to it. > > > > Signed-off-by: Pavel Reichl <preichl@redhat.com> > > With that fixed, > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --D > > > --- > > mkfs/xfs_mkfs.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > > index 01d2e8ca..a37d6848 100644 > > --- a/mkfs/xfs_mkfs.c > > +++ b/mkfs/xfs_mkfs.c > > @@ -1714,6 +1714,7 @@ naming_opts_parser( > > } else { > > cli->sb_feat.dir_version = getnum(value, opts, subopt); > > } > > + free((char *)value); ...well, that, and the ^^^^ cast here isn't necessary. --D > > break; > > case N_FTYPE: > > cli->sb_feat.dirftype = getnum(value, opts, subopt); > > -- > > 2.36.1 > >
On 5/24/22 23:08, Darrick J. Wong wrote: > On Tue, May 24, 2022 at 02:06:44PM -0700, Darrick J. Wong wrote: >> On Tue, May 24, 2022 at 10:40:40PM +0200, Pavel Reichl wrote: >>> 'value' is allocated by strup() in getstr(). It >> Nit: strdup, not strup. >> >>> needs to be freed as we do not keep any permanent >>> reference to it. >>> >>> Signed-off-by: Pavel Reichl <preichl@redhat.com> >> With that fixed, >> Reviewed-by: Darrick J. Wong <djwong@kernel.org> >> >> --D >> >>> --- >>> mkfs/xfs_mkfs.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c >>> index 01d2e8ca..a37d6848 100644 >>> --- a/mkfs/xfs_mkfs.c >>> +++ b/mkfs/xfs_mkfs.c >>> @@ -1714,6 +1714,7 @@ naming_opts_parser( >>> } else { >>> cli->sb_feat.dir_version = getnum(value, opts, subopt); >>> } >>> + free((char *)value); > ...well, that, and the ^^^^ cast here isn't necessary. > > --D Hi, thanks for the comment, but w/o the cast I get this warning xfs_mkfs.c:1717:22: warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 1717 | free(value); | ^~~~~ In file included from ../include/platform_defs.h:16, from ../include/libxfs.h:11, from xfs_mkfs.c:7: /usr/include/stdlib.h:555:25: note: expected ‘void *’ but argument is of type ‘const char *’ 555 | extern void free (void *__ptr) __THROW; > >>> break; >>> case N_FTYPE: >>> cli->sb_feat.dirftype = getnum(value, opts, subopt); >>> -- >>> 2.36.1 >>>
On Tue, May 24, 2022 at 11:13:01PM +0200, Pavel Reichl wrote: > > On 5/24/22 23:08, Darrick J. Wong wrote: > > On Tue, May 24, 2022 at 02:06:44PM -0700, Darrick J. Wong wrote: > > > On Tue, May 24, 2022 at 10:40:40PM +0200, Pavel Reichl wrote: > > > > 'value' is allocated by strup() in getstr(). It > > > Nit: strdup, not strup. > > > > > > > needs to be freed as we do not keep any permanent > > > > reference to it. > > > > > > > > Signed-off-by: Pavel Reichl <preichl@redhat.com> > > > With that fixed, > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > > > > > --D > > > > > > > --- > > > > mkfs/xfs_mkfs.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > > > > index 01d2e8ca..a37d6848 100644 > > > > --- a/mkfs/xfs_mkfs.c > > > > +++ b/mkfs/xfs_mkfs.c > > > > @@ -1714,6 +1714,7 @@ naming_opts_parser( > > > > } else { > > > > cli->sb_feat.dir_version = getnum(value, opts, subopt); > > > > } > > > > + free((char *)value); > > ...well, that, and the ^^^^ cast here isn't necessary. > > > > --D > > Hi, > > thanks for the comment, but w/o the cast I get this warning > > xfs_mkfs.c:1717:22: warning: passing argument 1 of ‘free’ discards ‘const’ > qualifier from pointer target type [-Wdiscarded-qualifiers] > 1717 | free(value); > | ^~~~~ > In file included from ../include/platform_defs.h:16, > from ../include/libxfs.h:11, > from xfs_mkfs.c:7: > /usr/include/stdlib.h:555:25: note: expected ‘void *’ but argument is of > type ‘const char *’ > 555 | extern void free (void *__ptr) __THROW; Ah, ok. Ignore my comment then. :) --D > > > > > > > break; > > > > case N_FTYPE: > > > > cli->sb_feat.dirftype = getnum(value, opts, subopt); > > > > -- > > > > 2.36.1 > > > > >
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 01d2e8ca..a37d6848 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -1714,6 +1714,7 @@ naming_opts_parser( } else { cli->sb_feat.dir_version = getnum(value, opts, subopt); } + free((char *)value); break; case N_FTYPE: cli->sb_feat.dirftype = getnum(value, opts, subopt);
'value' is allocated by strup() in getstr(). It needs to be freed as we do not keep any permanent reference to it. Signed-off-by: Pavel Reichl <preichl@redhat.com> --- mkfs/xfs_mkfs.c | 1 + 1 file changed, 1 insertion(+)