* [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern
@ 2016-09-01 2:14 fgao
2016-09-01 2:26 ` Feng Gao
0 siblings, 1 reply; 5+ messages in thread
From: fgao @ 2016-09-01 2:14 UTC (permalink / raw)
To: pablo, kadlec, netfilter-devel, coreteam; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
The caller function "help" has already make sure the datalen could not be zero
before invoke find_pattern as a parameter by the following codes
if (dataoff >= skb->len) {
pr_debug("ftp: dataoff(%u) >= skblen(%u)\n", dataoff,
skb->len);
return NF_ACCEPT;
}
datalen = skb->len - dataoff;
And the latter codes "ends_in_nl = (fb_ptr[datalen - 1] == '\n');" use datalen
directly without checking if it is zero.
So it is unneccessary to check it in find_pattern too.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/netfilter/nf_conntrack_ftp.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index b6934b5..77f05ac 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -301,8 +301,6 @@ static int find_pattern(const char *data, size_t dlen,
size_t i = plen;
pr_debug("find_pattern `%s': dlen = %Zu\n", pattern, dlen);
- if (dlen == 0)
- return 0;
if (dlen <= plen) {
/* Short packet: try for partial? */
@@ -312,16 +310,6 @@ static int find_pattern(const char *data, size_t dlen,
}
if (strncasecmp(data, pattern, plen) != 0) {
-#if 0
- size_t i;
-
- pr_debug("ftp: string mismatch\n");
- for (i = 0; i < plen; i++) {
- pr_debug("ftp:char %u `%c'(%u) vs `%c'(%u)\n",
- i, data[i], data[i],
- pattern[i], pattern[i]);
- }
-#endif
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern
2016-09-01 2:14 [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern fgao
@ 2016-09-01 2:26 ` Feng Gao
2016-09-01 10:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Feng Gao @ 2016-09-01 2:26 UTC (permalink / raw)
To: Pablo Neira Ayuso, kadlec, Netfilter Developer Mailing List,
coreteam
On Thu, Sep 1, 2016 at 10:14 AM, <fgao@ikuai8.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> The caller function "help" has already make sure the datalen could not be zero
> before invoke find_pattern as a parameter by the following codes
>
> if (dataoff >= skb->len) {
> pr_debug("ftp: dataoff(%u) >= skblen(%u)\n", dataoff,
> skb->len);
> return NF_ACCEPT;
> }
> datalen = skb->len - dataoff;
>
> And the latter codes "ends_in_nl = (fb_ptr[datalen - 1] == '\n');" use datalen
> directly without checking if it is zero.
>
> So it is unneccessary to check it in find_pattern too.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> net/netfilter/nf_conntrack_ftp.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
> index b6934b5..77f05ac 100644
> --- a/net/netfilter/nf_conntrack_ftp.c
> +++ b/net/netfilter/nf_conntrack_ftp.c
> @@ -301,8 +301,6 @@ static int find_pattern(const char *data, size_t dlen,
> size_t i = plen;
>
> pr_debug("find_pattern `%s': dlen = %Zu\n", pattern, dlen);
> - if (dlen == 0)
> - return 0;
>
> if (dlen <= plen) {
> /* Short packet: try for partial? */
> @@ -312,16 +310,6 @@ static int find_pattern(const char *data, size_t dlen,
> }
>
> if (strncasecmp(data, pattern, plen) != 0) {
> -#if 0
> - size_t i;
> -
> - pr_debug("ftp: string mismatch\n");
> - for (i = 0; i < plen; i++) {
> - pr_debug("ftp:char %u `%c'(%u) vs `%c'(%u)\n",
> - i, data[i], data[i],
> - pattern[i], pattern[i]);
> - }
> -#endif
> return 0;
> }
>
> --
> 1.9.1
>
>
Sorry, I forget one point. I also remove some codes which are
commented out by #if0.
Regards
Feng
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern
2016-09-01 2:26 ` Feng Gao
@ 2016-09-01 10:20 ` Pablo Neira Ayuso
2016-09-01 11:01 ` Feng Gao
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-09-01 10:20 UTC (permalink / raw)
To: Feng Gao; +Cc: kadlec, Netfilter Developer Mailing List, coreteam
On Thu, Sep 01, 2016 at 10:26:08AM +0800, Feng Gao wrote:
> On Thu, Sep 1, 2016 at 10:14 AM, <fgao@ikuai8.com> wrote:
> > From: Gao Feng <fgao@ikuai8.com>
> >
> > The caller function "help" has already make sure the datalen could not be zero
> > before invoke find_pattern as a parameter by the following codes
> >
> > if (dataoff >= skb->len) {
> > pr_debug("ftp: dataoff(%u) >= skblen(%u)\n", dataoff,
> > skb->len);
> > return NF_ACCEPT;
> > }
> > datalen = skb->len - dataoff;
> >
> > And the latter codes "ends_in_nl = (fb_ptr[datalen - 1] == '\n');" use datalen
> > directly without checking if it is zero.
> >
> > So it is unneccessary to check it in find_pattern too.
> >
> > Signed-off-by: Gao Feng <fgao@ikuai8.com>
> > ---
> > net/netfilter/nf_conntrack_ftp.c | 12 ------------
> > 1 file changed, 12 deletions(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
> > index b6934b5..77f05ac 100644
> > --- a/net/netfilter/nf_conntrack_ftp.c
> > +++ b/net/netfilter/nf_conntrack_ftp.c
> > @@ -301,8 +301,6 @@ static int find_pattern(const char *data, size_t dlen,
> > size_t i = plen;
> >
> > pr_debug("find_pattern `%s': dlen = %Zu\n", pattern, dlen);
> > - if (dlen == 0)
> > - return 0;
> >
> > if (dlen <= plen) {
> > /* Short packet: try for partial? */
> > @@ -312,16 +310,6 @@ static int find_pattern(const char *data, size_t dlen,
> > }
> >
> > if (strncasecmp(data, pattern, plen) != 0) {
> > -#if 0
> > - size_t i;
> > -
> > - pr_debug("ftp: string mismatch\n");
> > - for (i = 0; i < plen; i++) {
> > - pr_debug("ftp:char %u `%c'(%u) vs `%c'(%u)\n",
> > - i, data[i], data[i],
> > - pattern[i], pattern[i]);
> > - }
> > -#endif
> > return 0;
> > }
> >
> > --
> > 1.9.1
> >
> >
>
> Sorry, I forget one point. I also remove some codes which are
> commented out by #if0.
Please, do this in a separated patch. Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern
2016-09-01 10:20 ` Pablo Neira Ayuso
@ 2016-09-01 11:01 ` Feng Gao
2016-09-01 11:08 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Feng Gao @ 2016-09-01 11:01 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: kadlec, Netfilter Developer Mailing List, coreteam
On Thu, Sep 1, 2016 at 6:20 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Sep 01, 2016 at 10:26:08AM +0800, Feng Gao wrote:
>> On Thu, Sep 1, 2016 at 10:14 AM, <fgao@ikuai8.com> wrote:
>> > From: Gao Feng <fgao@ikuai8.com>
>> >
>> > The caller function "help" has already make sure the datalen could not be zero
>> > before invoke find_pattern as a parameter by the following codes
>> >
>> > if (dataoff >= skb->len) {
>> > pr_debug("ftp: dataoff(%u) >= skblen(%u)\n", dataoff,
>> > skb->len);
>> > return NF_ACCEPT;
>> > }
>> > datalen = skb->len - dataoff;
>> >
>> > And the latter codes "ends_in_nl = (fb_ptr[datalen - 1] == '\n');" use datalen
>> > directly without checking if it is zero.
>> >
>> > So it is unneccessary to check it in find_pattern too.
>> >
>> > Signed-off-by: Gao Feng <fgao@ikuai8.com>
>> > ---
>> > net/netfilter/nf_conntrack_ftp.c | 12 ------------
>> > 1 file changed, 12 deletions(-)
>> >
>> > diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
>> > index b6934b5..77f05ac 100644
>> > --- a/net/netfilter/nf_conntrack_ftp.c
>> > +++ b/net/netfilter/nf_conntrack_ftp.c
>> > @@ -301,8 +301,6 @@ static int find_pattern(const char *data, size_t dlen,
>> > size_t i = plen;
>> >
>> > pr_debug("find_pattern `%s': dlen = %Zu\n", pattern, dlen);
>> > - if (dlen == 0)
>> > - return 0;
>> >
>> > if (dlen <= plen) {
>> > /* Short packet: try for partial? */
>> > @@ -312,16 +310,6 @@ static int find_pattern(const char *data, size_t dlen,
>> > }
>> >
>> > if (strncasecmp(data, pattern, plen) != 0) {
>> > -#if 0
>> > - size_t i;
>> > -
>> > - pr_debug("ftp: string mismatch\n");
>> > - for (i = 0; i < plen; i++) {
>> > - pr_debug("ftp:char %u `%c'(%u) vs `%c'(%u)\n",
>> > - i, data[i], data[i],
>> > - pattern[i], pattern[i]);
>> > - }
>> > -#endif
>> > return 0;
>> > }
>> >
>> > --
>> > 1.9.1
>> >
>> >
>>
>> Sorry, I forget one point. I also remove some codes which are
>> commented out by #if0.
>
> Please, do this in a separated patch. Thanks.
Done.
I thought the "#if 0" change is very tiny, and it did not deserve one commit.
Now I get it. It needs one separate commit, even though how tiny it is.
Best Regards
Feng
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern
2016-09-01 11:01 ` Feng Gao
@ 2016-09-01 11:08 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-09-01 11:08 UTC (permalink / raw)
To: Feng Gao; +Cc: kadlec, Netfilter Developer Mailing List, coreteam
On Thu, Sep 01, 2016 at 07:01:51PM +0800, Feng Gao wrote:
> On Thu, Sep 1, 2016 at 6:20 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Sep 01, 2016 at 10:26:08AM +0800, Feng Gao wrote:
> >> On Thu, Sep 1, 2016 at 10:14 AM, <fgao@ikuai8.com> wrote:
[...]
> >> Sorry, I forget one point. I also remove some codes which are
> >> commented out by #if0.
> >
> > Please, do this in a separated patch. Thanks.
>
> Done.
> I thought the "#if 0" change is very tiny, and it did not deserve one commit.
>
> Now I get it. It needs one separate commit, even though how tiny it is.
Better split patches into logical changes.
In general, if you end up describing two changes that don't relate
each other in the patch description, then this usually means you need
two patches.
Thanks for addressing my feedback!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-01 11:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-01 2:14 [PATCH nf-next] netfilter: ftp: Remove the useless dlen==0 condition check in find_pattern fgao
2016-09-01 2:26 ` Feng Gao
2016-09-01 10:20 ` Pablo Neira Ayuso
2016-09-01 11:01 ` Feng Gao
2016-09-01 11:08 ` 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).