Hi Jozsef,
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?
--
Stefano
