Linux Netfilter development
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: Jan Engelhardt <jengelh@computergmbh.de>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH 3/8] [NETFILTER]: rename NF_ARP to NFPROTO_ARP and assign a non-clashing value
Date: Wed, 09 Apr 2008 15:21:56 +0200	[thread overview]
Message-ID: <47FCC2F4.4030901@trash.net> (raw)
In-Reply-To: <alpine.LNX.1.10.0804091514550.4411@fbirervta.pbzchgretzou.qr>

Jan Engelhardt wrote:
> On Wednesday 2008-04-09 15:12, Patrick McHardy wrote
>>
>>>> The array declarations should then use NFPROTO_MAX + 1.
>>> Why +1 if MAX is ARP?
>> Because array indices start at zero.
>>
> 
> It's all whacked up, srsly.
> 
> #define AF_RXRPC 33
> #define AF_MAX   34
> 
> IMHO we should just:
> 
> enum {
> 	NFPROTO_FOO = AF_MAX,
> 	NFPROTO_BAR,
> 	NFPROTO_BAZ,
> 	NFPROTO_MAX,
> };
> 
> and use NFPROTO_MAX in lieu of AF_MAX where due.
> Putting that into place with ARP means:
> 
> enum {
> 	NFPROTO_ARP = AF_MAX,
> 	NFPROTO_MAX,
> };
> 
> and use NFPROTO_MAX. No __NFPROTO_MIN, no __NFPROTO_MAX, and
> less confusion overall. How about it? :)

The decoupling of netfilter supported protocols from AF values
makes sense.

However I think _MAX definitions actually having the value MAX + 1
is pretty poor style. When you see an array dimensioned as
[XYZ_MAX] you always have to check whether it is really the maximum
(and thus a bug) or maximum + 1. So I'd prefer to have MAX really
be the maximum and use max + 1 for arrays etc.





  reply	other threads:[~2008-04-09 13:21 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-08 15:31 [PATCH 1/8] [NETFILTER]: Rename ipt_recent to xt_recent Jan Engelhardt
2008-04-08 15:31 ` [PATCH 2/8] [NETFILTER]: xt_recent: IPv6 support Jan Engelhardt
2008-04-09 12:48   ` Patrick McHardy
2008-04-09 13:30     ` Patrick McHardy
2008-04-09 13:44       ` Jan Engelhardt
2008-04-09 13:48         ` Patrick McHardy
2008-04-08 15:31 ` [PATCH 3/8] [NETFILTER]: rename NF_ARP to NFPROTO_ARP and assign a non-clashing value Jan Engelhardt
2008-04-09 12:52   ` Patrick McHardy
2008-04-09 13:09     ` Jan Engelhardt
2008-04-09 13:12       ` Patrick McHardy
2008-04-09 13:17         ` Jan Engelhardt
2008-04-09 13:21           ` Patrick McHardy [this message]
2008-04-09 13:34             ` Jan Engelhardt
2008-04-09 13:42               ` Patrick McHardy
2008-04-09 13:48                 ` Jan Engelhardt
2008-04-09 13:51                   ` Patrick McHardy
2008-04-09 13:59                     ` Jan Engelhardt
2008-04-09 14:30                       ` Patrick McHardy
2008-04-09 14:39                         ` Jan Engelhardt
2008-04-09 14:57                           ` Patrick McHardy
2008-04-09 15:12                             ` Jan Engelhardt
2008-04-09 15:15                               ` Patrick McHardy
2008-04-09 16:31                                 ` Jan Engelhardt
2008-04-09 16:35                                   ` Patrick McHardy
2008-04-09 16:46                                     ` Jan Engelhardt
2008-04-09 16:50                                       ` Patrick McHardy
2008-04-09 16:59                                         ` Jan Engelhardt
2008-04-09 17:05                                           ` Patrick McHardy
2008-04-09 17:38                                             ` Jan Engelhardt
2008-04-10  1:10                                               ` Patrick McHardy
2008-04-08 15:31 ` [PATCH 4/8] [NETFILTER]: Implement AF_UNSPEC as a wildcard for extensions Jan Engelhardt
2008-04-08 15:31 ` [PATCH 5/8] [NETFILTER]: Give AF-independent extensions an arpt_ alias Jan Engelhardt
2008-04-08 15:31 ` [PATCH 6/8] [NETFILTER]: Make Ebtables use Xtables infrastructure Jan Engelhardt
2008-04-09 13:08   ` Patrick McHardy
2008-04-09 13:12     ` Jan Engelhardt
2008-04-09 16:52     ` Jan Engelhardt
2008-04-10 20:11     ` Bart De Schuymer
2008-04-10 20:52       ` Jan Engelhardt
2008-04-13  5:24         ` Patrick McHardy
2008-04-08 15:31 ` [PATCH 7/8] [NETFILTER]: Collapse tcpmss_reverse_mtu{4,6} into one function Jan Engelhardt
2008-04-15 12:51   ` Patrick McHardy
2008-04-15 13:57     ` Jan Engelhardt
2008-04-15 13:59       ` Patrick McHardy
2008-04-08 15:31 ` [PATCH 8/8] [NETFILTER]: Deploy a prefix_length-to-network mask mapping table Jan Engelhardt
2008-04-09 12:45 ` [PATCH 1/8] [NETFILTER]: Rename ipt_recent to xt_recent Patrick McHardy
2008-04-09 12:50   ` Jan Engelhardt
2008-04-09 12:53     ` Patrick McHardy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47FCC2F4.4030901@trash.net \
    --to=kaber@trash.net \
    --cc=jengelh@computergmbh.de \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox