Hi,
On Mon, 27 Aug 2018, Stefano Brivio wrote:
> On Fri, 24 Aug 2018 21:02:12 +0200 (CEST)
> Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> wrote:
>
> > > @@ -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!
>
> I dropped this check because, as Sven-Haegar mentioned, free() on a
> NULL pointer has no effect. Both C89 and C99 say:
>
> The free function causes the space pointed to by ptr to be
> deallocated, that is, made available for further allocation. If ptr
> is a null pointer, no action occurs.
>
> As a side note, this pattern is being gradually removed in the kernel,
> also with the help of ifnullfree.cocci.
>
> Should I maintain it for ipset coding style reasons?
No, I have just applied your patch. Thanks!
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
