| Message ID | 20230123081957.1380790-1-Ilia.Gavrilov@infotecs.ru |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: conntrack: remote a return value of the 'seq_print_acct' function. | expand |
On Mon, Jan 23, 2023 at 08:19:50AM +0000, Gavrilov Ilia wrote: > The static 'seq_print_acct' function always returns 0. > > Change the return value to 'void' and remove unnecessary checks. > > Found by InfoTeCS on behalf of Linux Verification Center > (linuxtesting.org) with SVACE. > > Fixes: 1ca9e41770cb ("netfilter: Remove uses of seq_<foo> return values") > Signed-off-by: Ilia.Gavrilov <Ilia.Gavrilov@infotecs.ru> > --- > net/netfilter/nf_conntrack_standalone.c | 26 ++++++++++--------------- > 1 file changed, 10 insertions(+), 16 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c > index 0250725e38a4..bee99d4bcf36 100644 > --- a/net/netfilter/nf_conntrack_standalone.c > +++ b/net/netfilter/nf_conntrack_standalone.c > @@ -275,22 +275,18 @@ static const char* l4proto_name(u16 proto) > return "unknown"; > } > > -static unsigned int > +static void > seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir) > { > - struct nf_conn_acct *acct; > - struct nf_conn_counter *counter; > + struct nf_conn_acct *acct = nf_conn_acct_find(ct); > > - acct = nf_conn_acct_find(ct); > - if (!acct) > - return 0; > - > - counter = acct->counter; > - seq_printf(s, "packets=%llu bytes=%llu ", > - (unsigned long long)atomic64_read(&counter[dir].packets), > - (unsigned long long)atomic64_read(&counter[dir].bytes)); > + if (acct) { > + struct nf_conn_counter *counter = acct->counter; > > - return 0; > + seq_printf(s, "packets=%llu bytes=%llu ", > + (unsigned long long)atomic64_read(&counter[dir].packets), > + (unsigned long long)atomic64_read(&counter[dir].bytes)); > + } The preferred linux kernel style is to perform if (check_error) return; In this case, this pattern should stay. acct = nf_conn_acct_find(ct); if (!acct) return; Thanks
С уважением, Илья Гаврилов Ведущий программист Отдел разработки АО "ИнфоТеКС" в г. Санкт-Петербург 127287, г. Москва, Старый Петровско-Разумовский проезд, дом 1/23, стр. 1 T: +7 495 737-61-92 ( доб. 4921) Ф: +7 495 737-72-78 Ilia.Gavrilov@infotecs.ru www.infotecs.ru On 1/23/23 16:28, Leon Romanovsky wrote: > On Mon, Jan 23, 2023 at 08:19:50AM +0000, Gavrilov Ilia wrote: >> The static 'seq_print_acct' function always returns 0. >> >> Change the return value to 'void' and remove unnecessary checks. >> >> Found by InfoTeCS on behalf of Linux Verification Center >> (linuxtesting.org) with SVACE. >> >> Fixes: 1ca9e41770cb ("netfilter: Remove uses of seq_<foo> return values") >> Signed-off-by: Ilia.Gavrilov <Ilia.Gavrilov@infotecs.ru> >> --- >> net/netfilter/nf_conntrack_standalone.c | 26 ++++++++++--------------- >> 1 file changed, 10 insertions(+), 16 deletions(-) >> >> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c >> index 0250725e38a4..bee99d4bcf36 100644 >> --- a/net/netfilter/nf_conntrack_standalone.c >> +++ b/net/netfilter/nf_conntrack_standalone.c >> @@ -275,22 +275,18 @@ static const char* l4proto_name(u16 proto) >> return "unknown"; >> } >> >> -static unsigned int >> +static void >> seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir) >> { >> -struct nf_conn_acct *acct; >> -struct nf_conn_counter *counter; >> +struct nf_conn_acct *acct = nf_conn_acct_find(ct); >> >> -acct = nf_conn_acct_find(ct); >> -if (!acct) >> -return 0; >> - >> -counter = acct->counter; >> -seq_printf(s, "packets=%llu bytes=%llu ", >> - (unsigned long long)atomic64_read(&counter[dir].packets), >> - (unsigned long long)atomic64_read(&counter[dir].bytes)); >> +if (acct) { >> +struct nf_conn_counter *counter = acct->counter; >> >> -return 0; >> +seq_printf(s, "packets=%llu bytes=%llu ", >> + (unsigned long long)atomic64_read(&counter[dir].packets), >> + (unsigned long long)atomic64_read(&counter[dir].bytes)); >> +} > > The preferred linux kernel style is to perform if (check_error) return; > In this case, this pattern should stay. > > acct = nf_conn_acct_find(ct); > if (!acct) > return; > > Thanks Thank you for review. I'll fix it in v2.
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c index 0250725e38a4..bee99d4bcf36 100644 --- a/net/netfilter/nf_conntrack_standalone.c +++ b/net/netfilter/nf_conntrack_standalone.c @@ -275,22 +275,18 @@ static const char* l4proto_name(u16 proto) return "unknown"; } -static unsigned int +static void seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir) { - struct nf_conn_acct *acct; - struct nf_conn_counter *counter; + struct nf_conn_acct *acct = nf_conn_acct_find(ct); - acct = nf_conn_acct_find(ct); - if (!acct) - return 0; - - counter = acct->counter; - seq_printf(s, "packets=%llu bytes=%llu ", - (unsigned long long)atomic64_read(&counter[dir].packets), - (unsigned long long)atomic64_read(&counter[dir].bytes)); + if (acct) { + struct nf_conn_counter *counter = acct->counter; - return 0; + seq_printf(s, "packets=%llu bytes=%llu ", + (unsigned long long)atomic64_read(&counter[dir].packets), + (unsigned long long)atomic64_read(&counter[dir].bytes)); + } } /* return 0 on success, 1 in case of error */ @@ -342,8 +338,7 @@ static int ct_seq_show(struct seq_file *s, void *v) if (seq_has_overflowed(s)) goto release; - if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL)) - goto release; + seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL); if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status))) seq_puts(s, "[UNREPLIED] "); @@ -352,8 +347,7 @@ static int ct_seq_show(struct seq_file *s, void *v) ct_show_zone(s, ct, NF_CT_ZONE_DIR_REPL); - if (seq_print_acct(s, ct, IP_CT_DIR_REPLY)) - goto release; + seq_print_acct(s, ct, IP_CT_DIR_REPLY); if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status)) seq_puts(s, "[HW_OFFLOAD] ");
The static 'seq_print_acct' function always returns 0. Change the return value to 'void' and remove unnecessary checks. Found by InfoTeCS on behalf of Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1ca9e41770cb ("netfilter: Remove uses of seq_<foo> return values") Signed-off-by: Ilia.Gavrilov <Ilia.Gavrilov@infotecs.ru> --- net/netfilter/nf_conntrack_standalone.c | 26 ++++++++++--------------- 1 file changed, 10 insertions(+), 16 deletions(-)