Re: [PATCH 14/24] [NETFILTER]: Use bool in nf_conntrack_l4proto

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jan Engelhardt wrote:
On Thursday 2008-04-03 17:00, Patrick McHardy wrote:

Some hints for the future to make this easier for both of us:

- submit small batches, split into logical units

This sets me up a bit.. sometimes it's "should have folded these"
(like the const annotation patches that were at the start of
the series), then it's "smaller batches" :-/

Folding patches makes the batches smaller (note: batch, not
patch) :) What I meant by this was to not flood me with 30
patches three times at once, if issues come up in one of the
first patches it often results in the later ones not applying.
Logical batches, like

- batch 1: constification
- batch 2: boolean conversions
- batch x: things like rename ipt_recent, add IPv6 support
- batch y: arp_tables userspace interface changes in order
           to achieve X.

- avoid style changes like this, especially when the entire file
  uses a consistent style:

 -static int icmpv6_pkt_to_tuple(const struct sk_buff *skb,
 -                   unsigned int dataoff,
 -                   struct nf_conntrack_tuple *tuple)

allow me the remark of "consistently odd", as you can see what a
change in indent does when spaces are not used in the right
place. But whatever, yeah.

 +static bool
 +icmpv6_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
 +                    struct nf_conntrack_tuple *tuple)

- run checkpatch

Not before I rip out that incredibly stupid "use tabs" warning.
The warning may be right for users who apparently have not dealt
with patch submitting process a lot, but for longtime contributers
that get their style right the 1st time it's just wrong.

No, we've fixed up net/ more than once using scripts before
checkpatch even existed. Simple: keep existing style.

    That program has no sense for when spaces are needed.
The style I used and use (tabs=indent, spaces=align - it only makes
sense) was always fine by you, but now it's not because
checkpatch says so?

See above. I used to use a different indenting style as well,
but I prefer consistency over having it exactly as I like it.

- don't mix cleanups with real changes
- run sparse with endian checks
- compile test all the code your changing, including CONFIG_COMPAT

Will do.


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux