* [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c @ 2017-01-11 1:32 fgao 2017-01-12 11:21 ` Pablo Neira Ayuso 0 siblings, 1 reply; 4+ messages in thread From: fgao @ 2017-01-11 1:32 UTC (permalink / raw) To: pablo, netfilter-devel; +Cc: gfree.wind, Gao Feng From: Gao Feng <fgao@ikuai8.com> The return value of nf_tables_table_lookup is valid pointer or one pointer error. There are two cases totally. case1: IS_ERR(table) is true, it would return the error or reset the table as NULL, it is unnecessary to perform the latter check "table != NULL". case2: IS_ERR(obj) is false, the table is one valid pointer. It is also unnecessary to perform that check. The nf_tables_newset and nf_tables_newobj have same logic codes. In summary, we could move the block of condition check "table != NULL" in the else block to eliminate the original condition checks. Signed-off-by: Gao Feng <fgao@ikuai8.com> --- net/netfilter/nf_tables_api.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index a019a87..3d7267f 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -697,9 +697,7 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, if (PTR_ERR(table) != -ENOENT) return PTR_ERR(table); table = NULL; - } - - if (table != NULL) { + } else { if (nlh->nlmsg_flags & NLM_F_EXCL) return -EEXIST; if (nlh->nlmsg_flags & NLM_F_REPLACE) @@ -2964,9 +2962,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, if (PTR_ERR(set) != -ENOENT) return PTR_ERR(set); set = NULL; - } - - if (set != NULL) { + } else { if (nlh->nlmsg_flags & NLM_F_EXCL) return -EEXIST; if (nlh->nlmsg_flags & NLM_F_REPLACE) @@ -4154,9 +4150,7 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk, return err; obj = NULL; - } - - if (obj != NULL) { + } else { if (nlh->nlmsg_flags & NLM_F_EXCL) return -EEXIST; -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c 2017-01-11 1:32 [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c fgao @ 2017-01-12 11:21 ` Pablo Neira Ayuso 2017-01-12 15:10 ` Gao Feng 0 siblings, 1 reply; 4+ messages in thread From: Pablo Neira Ayuso @ 2017-01-12 11:21 UTC (permalink / raw) To: fgao; +Cc: netfilter-devel, gfree.wind On Wed, Jan 11, 2017 at 09:32:15AM +0800, fgao@ikuai8.com wrote: > From: Gao Feng <fgao@ikuai8.com> > > The return value of nf_tables_table_lookup is valid pointer or one > pointer error. There are two cases totally. > case1: IS_ERR(table) is true, it would return the error or reset the > table as NULL, it is unnecessary to perform the latter check > "table != NULL". > case2: IS_ERR(obj) is false, the table is one valid pointer. It is also > unnecessary to perform that check. > The nf_tables_newset and nf_tables_newobj have same logic codes. > > In summary, we could move the block of condition check "table != NULL" > in the else block to eliminate the original condition checks. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > net/netfilter/nf_tables_api.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index a019a87..3d7267f 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -697,9 +697,7 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, > if (PTR_ERR(table) != -ENOENT) > return PTR_ERR(table); > table = NULL; We follow up with table = NULL down the code, I think this breaks. > - } > - > - if (table != NULL) { > + } else { > if (nlh->nlmsg_flags & NLM_F_EXCL) > return -EEXIST; > if (nlh->nlmsg_flags & NLM_F_REPLACE) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c 2017-01-12 11:21 ` Pablo Neira Ayuso @ 2017-01-12 15:10 ` Gao Feng 2017-01-16 13:38 ` Pablo Neira Ayuso 0 siblings, 1 reply; 4+ messages in thread From: Gao Feng @ 2017-01-12 15:10 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Netfilter Developer Mailing List Hi Pablo, On Thu, Jan 12, 2017 at 7:21 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Wed, Jan 11, 2017 at 09:32:15AM +0800, fgao@ikuai8.com wrote: >> From: Gao Feng <fgao@ikuai8.com> >> >> The return value of nf_tables_table_lookup is valid pointer or one >> pointer error. There are two cases totally. >> case1: IS_ERR(table) is true, it would return the error or reset the >> table as NULL, it is unnecessary to perform the latter check >> "table != NULL". >> case2: IS_ERR(obj) is false, the table is one valid pointer. It is also >> unnecessary to perform that check. >> The nf_tables_newset and nf_tables_newobj have same logic codes. >> >> In summary, we could move the block of condition check "table != NULL" >> in the else block to eliminate the original condition checks. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> >> --- >> net/netfilter/nf_tables_api.c | 12 +++--------- >> 1 file changed, 3 insertions(+), 9 deletions(-) >> >> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c >> index a019a87..3d7267f 100644 >> --- a/net/netfilter/nf_tables_api.c >> +++ b/net/netfilter/nf_tables_api.c >> @@ -697,9 +697,7 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, >> if (PTR_ERR(table) != -ENOENT) >> return PTR_ERR(table); >> table = NULL; > > We follow up with table = NULL down the code, I think this breaks. Look at the following nf_tables_table_lookup codes, it won't return NULL. It returns one valid table pointer or one error. static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi, const struct nlattr *nla, u8 genmask) { struct nft_table *table; if (nla == NULL) return ERR_PTR(-EINVAL); table = nft_table_lookup(afi, nla, genmask); if (table != NULL) return table; return ERR_PTR(-ENOENT); } When returns one error, IS_ERR(table) is true, it then returns error or reset table as NULL. Wehn returns a valid table pointer, IS_ERR(table) is false, we could perform the latter codes like "if (nlh->nlmsg_flags & NLM_F_EXCL)" directly. Regards Feng > >> - } >> - >> - if (table != NULL) { >> + } else { >> if (nlh->nlmsg_flags & NLM_F_EXCL) >> return -EEXIST; >> if (nlh->nlmsg_flags & NLM_F_REPLACE) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c 2017-01-12 15:10 ` Gao Feng @ 2017-01-16 13:38 ` Pablo Neira Ayuso 0 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2017-01-16 13:38 UTC (permalink / raw) To: Gao Feng; +Cc: Netfilter Developer Mailing List On Thu, Jan 12, 2017 at 11:10:11PM +0800, Gao Feng wrote: > Hi Pablo, > > On Thu, Jan 12, 2017 at 7:21 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Wed, Jan 11, 2017 at 09:32:15AM +0800, fgao@ikuai8.com wrote: > >> From: Gao Feng <fgao@ikuai8.com> > >> > >> The return value of nf_tables_table_lookup is valid pointer or one > >> pointer error. There are two cases totally. > >> case1: IS_ERR(table) is true, it would return the error or reset the > >> table as NULL, it is unnecessary to perform the latter check > >> "table != NULL". > >> case2: IS_ERR(obj) is false, the table is one valid pointer. It is also > >> unnecessary to perform that check. > >> The nf_tables_newset and nf_tables_newobj have same logic codes. > >> > >> In summary, we could move the block of condition check "table != NULL" > >> in the else block to eliminate the original condition checks. > >> > >> Signed-off-by: Gao Feng <fgao@ikuai8.com> > >> --- > >> net/netfilter/nf_tables_api.c | 12 +++--------- > >> 1 file changed, 3 insertions(+), 9 deletions(-) > >> > >> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > >> index a019a87..3d7267f 100644 > >> --- a/net/netfilter/nf_tables_api.c > >> +++ b/net/netfilter/nf_tables_api.c > >> @@ -697,9 +697,7 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, > >> if (PTR_ERR(table) != -ENOENT) > >> return PTR_ERR(table); > >> table = NULL; > > > > We follow up with table = NULL down the code, I think this breaks. > > Look at the following nf_tables_table_lookup codes, it won't return NULL. > It returns one valid table pointer or one error. > > static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi, > const struct nlattr *nla, > u8 genmask) > { > struct nft_table *table; > > if (nla == NULL) > return ERR_PTR(-EINVAL); > > table = nft_table_lookup(afi, nla, genmask); > if (table != NULL) > return table; > > return ERR_PTR(-ENOENT); > } > > When returns one error, IS_ERR(table) is true, it then returns error > or reset table as NULL. > Wehn returns a valid table pointer, IS_ERR(table) is false, we could > perform the latter codes like "if (nlh->nlmsg_flags & NLM_F_EXCL)" > directly. Right. Then, I think we can remove the unneccessary table = NULL assigment: @@ -697,9 +697,7 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, if (PTR_ERR(table) != -ENOENT) return PTR_ERR(table); table = NULL; <----- this - } - - if (table != NULL) { + } else { if (nlh->nlmsg_flags & NLM_F_EXCL) return -EEXIST; if (nlh->nlmsg_flags & NLM_F_REPLACE) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-16 13:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-11 1:32 [PATCH nf-next 1/1] netfilter: nf_tables: Refine the codes to eliminate useless condition checks in nf_tables_api.c fgao 2017-01-12 11:21 ` Pablo Neira Ayuso 2017-01-12 15:10 ` Gao Feng 2017-01-16 13:38 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).