[nft PATCH] mnl: Remove dead code

[nft PATCH] mnl: Remove dead code

[Phil Sutter]

Remove the functions mnl_nft_rule_add() and *_delete() since they are
not used throughout the code. Commit a72315d2bad47 ("src: add rule
batching support") changed their only caller to use the batch variant
introduced at the same time.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/mnl.h |  4 ----
 src/mnl.c     | 28 ----------------------------
 2 files changed, 32 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 9f5b34f60d023..3d2d7fef93ba2 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -33,10 +33,6 @@ int mnl_nft_rule_batch_del(struct nftnl_rule *nlr, struct nftnl_batch *batch,
 int mnl_nft_rule_batch_replace(struct nftnl_rule *nlr, struct nftnl_batch *batch,
 			       unsigned int flags, uint32_t seqnum);
 
-int mnl_nft_rule_add(struct mnl_socket *nf_sock, struct nftnl_rule *r,
-		     unsigned int flags);
-int mnl_nft_rule_delete(struct mnl_socket *nf_sock, struct nftnl_rule *r,
-			unsigned int flags);
 struct nftnl_rule_list *mnl_nft_rule_dump(struct mnl_socket *nf_sock,
 					int family);
 
diff --git a/src/mnl.c b/src/mnl.c
index 7639312381507..3db80de6da02d 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -339,34 +339,6 @@ int mnl_nft_rule_batch_del(struct nftnl_rule *nlr, struct nftnl_batch *batch,
 /*
  * Rule
  */
-int mnl_nft_rule_add(struct mnl_socket *nf_sock, struct nftnl_rule *nlr,
-		     unsigned int flags)
-{
-	char buf[MNL_SOCKET_BUFFER_SIZE];
-	struct nlmsghdr *nlh;
-
-	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_NEWRULE,
-				    nftnl_rule_get_u32(nlr, NFTNL_RULE_FAMILY),
-				    NLM_F_ACK | NLM_F_CREATE | flags, seq);
-	nftnl_rule_nlmsg_build_payload(nlh, nlr);
-
-	return nft_mnl_talk(nf_sock, nlh, nlh->nlmsg_len, NULL, NULL);
-}
-
-int mnl_nft_rule_delete(struct mnl_socket *nf_sock, struct nftnl_rule *nlr,
-			unsigned int flags)
-{
-	char buf[MNL_SOCKET_BUFFER_SIZE];
-	struct nlmsghdr *nlh;
-
-	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_DELRULE,
-				    nftnl_rule_get_u32(nlr, NFTNL_RULE_FAMILY),
-				    NLM_F_ACK, seq);
-	nftnl_rule_nlmsg_build_payload(nlh, nlr);
-
-	return nft_mnl_talk(nf_sock, nlh, nlh->nlmsg_len, NULL, NULL);
-}
-
 static int rule_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nftnl_rule_list *nlr_list = data;
-- 
2.13.1

Re: [nft PATCH] mnl: Remove dead code

[Pablo Neira Ayuso]

Cc'ing Pablo Bermudo.

On Wed, Jul 26, 2017 at 06:16:30PM +0200, Phil Sutter wrote:
> Remove the functions mnl_nft_rule_add() and *_delete() since they are
> not used throughout the code. Commit a72315d2bad47 ("src: add rule
> batching support") changed their only caller to use the batch variant
> introduced at the same time.

I think there are more useless functions. Pablo Bermudo mentioned this
to me a while ago. At least I remember some of them in netlink.c?

If not much asking, it would be good if we can kill them all in one go.

Re: [nft PATCH] mnl: Remove dead code

[Pablo M. Bermudo Garay]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

2017-07-27 10:46 GMT+02:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> Cc'ing Pablo Bermudo.
>
> On Wed, Jul 26, 2017 at 06:16:30PM +0200, Phil Sutter wrote:
>> Remove the functions mnl_nft_rule_add() and *_delete() since they are
>> not used throughout the code. Commit a72315d2bad47 ("src: add rule
>> batching support") changed their only caller to use the batch variant
>> introduced at the same time.
>
> I think there are more useless functions. Pablo Bermudo mentioned this
> to me a while ago. At least I remember some of them in netlink.c?
>
> If not much asking, it would be good if we can kill them all in one go.

I am using a combination of compiler and linker options in order to obtain
a list of unused functions:

  $ make -j5 CFLAGS="-ffunction-sections -fdata-sections
-Wl,--gc-sections,--print-gc-sections"

You can find more information about --gc-sections and its pitfalls here [1].
The listing is printed on stderr by --print-gc-sections and it looks like this:

/usr/bin/ld: Removing unused section '.text.xt_stmt_xlate' in file 'statement.o'
/usr/bin/ld: Removing unused section '.text.xt_stmt_release' in file
'statement.o'
/usr/bin/ld: Removing unused section '.text.xt_stmt_print' in file 'statement.o'
...

There is also a small utility called callcatcher [2], written by LibreOffice
developer Caolán McNamara. But it seems not to be recursive, so unused
functions invoked from other unused functions are not shown until the callers
are deleted.

The complete list contains some functions that I think are not worth removing,
because are part of a "consolidated API" (e.g. those related to the
implementation of the red–black tree). A bunch of unused functions are
from the auto-generated file scanner.c.

The current list is attached.

--

[1] http://elinux.org/images/2/2d/ELC2010-gc-sections_Denys_Vlasenko.pdf
[2] http://www.skynet.ie/~caolan/Packages/callcatcher.html

[-- Attachment #2: nft-unused-code --]
[-- Type: application/octet-stream, Size: 1770 bytes --]

# Unused struct
xt_stmt_ops             in file 'statement.c'

# Unused functions
chain_get_cb            in file 'mnl.c'
cli_display             in file 'cli.c'
mnl_nft_chain_get       in file 'mnl.c'
mnl_nft_rule_add        in file 'mnl.c'
mnl_nft_rule_delete     in file 'mnl.c'
mnl_nft_set_get         in file 'mnl.c'
mnl_nft_table_get       in file 'mnl.c'
mpz_get_be64            in file 'gmputil.c'
netlink_add_rule_list   in file 'netlink.c'
netlink_dump_table      in file 'netlink.c'
netlink_get_chain       in file 'netlink.c'
netlink_get_set         in file 'netlink.c'
netlink_get_table       in file 'netlink.c'
netlink_list_chain      in file 'netlink.c'
nft_set_lval            in file 'scanner.c'
rb_last                 in file 'rbtree.c'
rb_prev                 in file 'rbtree.c'
rb_replace_node         in file 'rbtree.c'
set_get_cb              in file 'mnl.c'
table_get_cb            in file 'mnl.c'
xt_stmt_alloc           in file 'statement.c'
xt_stmt_destroy         in file 'statement.c'
xt_stmt_print           in file 'statement.c'
xt_stmt_release         in file 'statement.c'
xt_stmt_xlate           in file 'statement.c'

nft_get_column          in file 'scanner.c'
nft_get_debug           in file 'scanner.c'
nft_get_in              in file 'scanner.c'
nft_get_leng            in file 'scanner.c'
nft_get_lineno          in file 'scanner.c'
nft_get_lloc            in file 'scanner.c'
nft_get_lval            in file 'scanner.c'
nft_get_out             in file 'scanner.c'
nft_get_text            in file 'scanner.c'
nft_lex_init_extra      in file 'scanner.c'
nft_set_column          in file 'scanner.c'
nft_set_in              in file 'scanner.c'
nft_set_lineno          in file 'scanner.c'
nft_set_lloc            in file 'scanner.c'