mbox series

[nft,v2,0/4,RESENT] remove xfree() and add free_const()+nft_gmp_free()

Message ID 20231024095820.1068949-1-thaller@redhat.com
Headers show
Series remove xfree() and add free_const()+nft_gmp_free() | expand

Message

Thomas Haller Oct. 24, 2023, 9:57 a.m. UTC
RESENT of v1.

Also rebased on top of current `master`, which required minor
adjustments.

Also minor adjustments to the commit messages.

Thomas Haller (4):
  datatype: don't return a const string from cgroupv2_get_path()
  gmputil: add nft_gmp_free() to free strings from mpz_get_str()
  all: add free_const() and use it instead of xfree()
  all: remove xfree() and use plain free()

 include/gmputil.h       |   2 +
 include/nft.h           |   6 ++
 include/utils.h         |   1 -
 src/cache.c             |   6 +-
 src/ct.c                |   2 +-
 src/datatype.c          |  18 ++---
 src/erec.c              |   6 +-
 src/evaluate.c          |  18 ++---
 src/expression.c        |   6 +-
 src/gmputil.c           |  21 +++++-
 src/json.c              |   2 +-
 src/libnftables.c       |  24 +++---
 src/meta.c              |   4 +-
 src/misspell.c          |   2 +-
 src/mnl.c               |  16 ++--
 src/netlink_linearize.c |   4 +-
 src/optimize.c          |  12 +--
 src/parser_bison.y      | 158 ++++++++++++++++++++--------------------
 src/rule.c              |  68 ++++++++---------
 src/scanner.l           |   6 +-
 src/segtree.c           |   4 +-
 src/statement.c         |   4 +-
 src/utils.c             |   5 --
 src/xt.c                |  10 +--
 24 files changed, 213 insertions(+), 192 deletions(-)

Comments

Pablo Neira Ayuso Nov. 6, 2023, 1:35 p.m. UTC | #1
On Tue, Oct 24, 2023 at 11:57:06AM +0200, Thomas Haller wrote:
> RESENT of v1.
> 
> Also rebased on top of current `master`, which required minor
> adjustments.
> 
> Also minor adjustments to the commit messages.

I will put this in the tree this evening, after with the recent fixes
I have posted.

> Thomas Haller (4):
>   datatype: don't return a const string from cgroupv2_get_path()
>   gmputil: add nft_gmp_free() to free strings from mpz_get_str()
>   all: add free_const() and use it instead of xfree()
>   all: remove xfree() and use plain free()
> 
>  include/gmputil.h       |   2 +
>  include/nft.h           |   6 ++
>  include/utils.h         |   1 -
>  src/cache.c             |   6 +-
>  src/ct.c                |   2 +-
>  src/datatype.c          |  18 ++---
>  src/erec.c              |   6 +-
>  src/evaluate.c          |  18 ++---
>  src/expression.c        |   6 +-
>  src/gmputil.c           |  21 +++++-
>  src/json.c              |   2 +-
>  src/libnftables.c       |  24 +++---
>  src/meta.c              |   4 +-
>  src/misspell.c          |   2 +-
>  src/mnl.c               |  16 ++--
>  src/netlink_linearize.c |   4 +-
>  src/optimize.c          |  12 +--
>  src/parser_bison.y      | 158 ++++++++++++++++++++--------------------
>  src/rule.c              |  68 ++++++++---------
>  src/scanner.l           |   6 +-
>  src/segtree.c           |   4 +-
>  src/statement.c         |   4 +-
>  src/utils.c             |   5 --
>  src/xt.c                |  10 +--
>  24 files changed, 213 insertions(+), 192 deletions(-)
> 
> -- 
> 2.41.0
>
Pablo Neira Ayuso Nov. 9, 2023, 4:05 p.m. UTC | #2
On Tue, Oct 24, 2023 at 11:57:06AM +0200, Thomas Haller wrote:
> RESENT of v1.
> 
> Also rebased on top of current `master`, which required minor
> adjustments.
> 
> Also minor adjustments to the commit messages.

Applied.

To be honest, reading the longish commit descriptions several times, I
doubt there is any benefit in this, I might end up myself using
free_const() everywhere not to figure out if it is const or not,
because I don't really care.

But now this series are in master.
Thomas Haller Nov. 9, 2023, 5:14 p.m. UTC | #3
On Thu, 2023-11-09 at 17:05 +0100, Pablo Neira Ayuso wrote:
> On Tue, Oct 24, 2023 at 11:57:06AM +0200, Thomas Haller wrote:
> > RESENT of v1.
> > 
> > Also rebased on top of current `master`, which required minor
> > adjustments.
> > 
> > Also minor adjustments to the commit messages.
> 
> Applied.

Thanks.

> 
> To be honest, reading the longish commit descriptions several times,
> I
> doubt there is any benefit in this, 

I claim, there was no benefit in xfree(). that's why it was dropped in
favor of standard free(). The special function xfree() needs a
justification, not the other way around.


> I might end up myself using
> free_const() everywhere not to figure out if it is const or not,
> because I don't really care.

That seems not a good practice. Const-correctness may help you to catch
bugs via unwanted modifications. If constness is unnecessarily cast
away, it's looses such hints from the compiler.


Thomas
Pablo Neira Ayuso Nov. 9, 2023, 7:15 p.m. UTC | #4
On Thu, Nov 09, 2023 at 06:14:56PM +0100, Thomas Haller wrote:
> On Thu, 2023-11-09 at 17:05 +0100, Pablo Neira Ayuso wrote:
[...]
> > I might end up myself using
> > free_const() everywhere not to figure out if it is const or not,
> > because I don't really care.
> 
> That seems not a good practice. Const-correctness may help you to catch
> bugs via unwanted modifications. If constness is unnecessarily cast
> away, it's looses such hints from the compiler.

Why should I care if the pointer is const or not if what I need to
free it?
Thomas Haller Nov. 9, 2023, 8:02 p.m. UTC | #5
On Thu, 2023-11-09 at 20:15 +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 09, 2023 at 06:14:56PM +0100, Thomas Haller wrote:
> > On Thu, 2023-11-09 at 17:05 +0100, Pablo Neira Ayuso wrote:
> [...]
> > > I might end up myself using
> > > free_const() everywhere not to figure out if it is const or not,
> > > because I don't really care.
> > 
> > That seems not a good practice. Const-correctness may help you to
> > catch
> > bugs via unwanted modifications. If constness is unnecessarily cast
> > away, it's looses such hints from the compiler.
> 
> Why should I care if the pointer is const or not if what I need to
> free it?

There might be a mistake/bug, and the thing should not actually be
freed.

We need every help from the compiler we can get (compiler warnings,
strong typing/const-correctness, static assertions).

Normally, the compiler would help and complain against free() of a
const pointer. With xfree()/free_const() used eagerly, it does not
help.


Thomas