On Fri, 24 Aug 2018, Jozsef Kadlecsik wrote:
> Hi Stefano,
>
> 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.
c'ya
sven-haegar
--
Three may keep a secret, if two of them are dead.
- Ben F.
