答复: 答复: [PATCH 1/1] netfilter: Add helper array register/unregister functions

From: 高峰 <fgao@ikuai8.com.aqb.so>
To: "'Pablo Neira Ayuso'" <pablo@netfilter.org>
Cc: <kaber@trash.net>, <netfilter-devel@vger.kernel.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<gfree.wind@gmail.com>
Subject: 答复: 答复: [PATCH 1/1] netfilter: Add helper array register/unregister functions
Date: Wed, 20 Jul 2016 16:57:32 +0800	[thread overview]
Message-ID: <016801d1e264$c194b390$44be1ab0$@ikuai8.com> (raw)
In-Reply-To: <20160720085018.GD1336@salvia>

Ok, I get it. 
Thanks Pablo.

Another question, Liping found the port style of sip, irc, and tftp was different with ftp.
It should be use iterator "i" to generate name for them.
Then should I update the original patch or create another one to correct it?

BTW, I am sorry about that. I didn't notice there are two port styles.

-----邮件原件-----
发件人: Pablo Neira Ayuso [mailto:pablo@netfilter.org] 
发送时间: 2016年7月20日 16:50
收件人: 高峰 <fgao@ikuai8.com>
抄送: kaber@trash.net; netfilter-devel@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; gfree.wind@gmail.com
主题: Re: 答复: [PATCH 1/1] netfilter: Add helper array register/unregister functions

On Wed, Jul 20, 2016 at 08:31:13AM +0800, 高峰 wrote:
> Thanks Pablo.
> 
> I had used the script "checkpatch.pl" to check the patch file.
> There was no indentation error reported.
> 
> So could you give me more tails please or point one indentation error?
> Then I could correct it by myself next time.

I'm refering to this specifically:

static int function(int parameter1, struct another_structure *blah,
                    int parameter2, unsigned int parameter3);
                    ^

It is just a comestic issue, but we consistently align function parameters to the initial parens.

As I said, I have just manually fixed this here, so no problem, just keep this in mind for the next time.

Another observation: You should bump patch version numbering in each revision and keep some history on its evolution.

The area after the patch separator --- and before diff stats is good to place volatile information that is only meaningful to the review process, I mean something like this:

  subsystem: Patch title.

  Patch description...

  Signed-off-by: Lucas Skywalker <trotacielos@blackstar.org>
  ---
  v3: Address comments from Chebakia on possible backward compatibility
      issues.
  v2: New parameter to control something.
  v1: Initial patch.

  include/net/netfilter/nf_tables.h |  25 ++-
  ...
  diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h

Thanks.