On Sat, 25 Aug 2018, Sven-Haegar Koch wrote:
> > On Wed, 22 Aug 2018, Stefano Brivio wrote:
> >
> > > When check_setname is used in ipset_parse_name_compat(), the
> > > 'str' and 'saved' macro arguments point in fact to the same
> > > buffer. Free the 'saved' argument only after using it.
> > >
> > > While at it, remove a useless NULL check on 'saved'.
> > >
> > > Signed-off-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
> > > ---
> > > lib/parse.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/parse.c b/lib/parse.c
> > > index 9a79ccda796c..4963d519c631 100644
> > > --- a/lib/parse.c
> > > +++ b/lib/parse.c
> > > @@ -1396,10 +1396,11 @@ ipset_parse_iptimeout(struct ipset_session *session,
> > > #define check_setname(str, saved) \
> > > do { \
> > > if (strlen(str) > IPSET_MAXNAMELEN - 1) { \
> > > - if (saved != NULL) \
> > > - free(saved); \
> > > - return syntax_err("setname '%s' is longer than %u characters",\
> > > + int err; \
> > > + err = syntax_err("setname '%s' is longer than %u characters",\
> > > str, IPSET_MAXNAMELEN - 1); \
> > > + free(saved); \
> > > + return err; \
> > > } \
> > > } while (0)
> >
> > At some places check_setname() is invoked with "saved" explicitly set to
> > NULL, so the condition to free cannot be left out. Please resubmit a new
> > version of the patch. Thanks!
>
> Why?
>
> The free() manpage says:
>
> If ptr is NULL, no operation is performed.
>
> So it is ok to always call it on a NULL pointer. The same as with
> kfree() in the kernel, no extra NULL check needed before.
You are right. I suppose old customs die slowly :-) (in old times it was
not proper to call kfree() with a NULL pointer).
Best regards,
Jozsef
-
E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
