All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cong Wang <amwang@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, tmb@mageia.org,
	libc-alpha@sourceware.org, yoshfuji@linux-ipv6.org,
	carlos@redhat.com
Subject: Re: [Patch net-next] net: sync some IP headers with glibc
Date: Thu, 15 Aug 2013 10:48:57 +0800	[thread overview]
Message-ID: <1376534937.2626.6.camel@cr0> (raw)
In-Reply-To: <20130814.134246.1152657041177088716.davem@davemloft.net>

On Wed, 2013-08-14 at 13:42 -0700, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Tue, 13 Aug 2013 16:37:33 +0800
> 
> > @@ -23,31 +23,54 @@
> >  
> >  /* Standard well-defined IP protocols.  */
> >  enum {
> > -  IPPROTO_IP = 0,		/* Dummy protocol for TCP		*/
> > -  IPPROTO_ICMP = 1,		/* Internet Control Message Protocol	*/
> 
> > +  IPPROTO_IP = 0,		/* Dummy protocol for TCP. */
> 
> Please don't do unrelated things like reformat comments, it makes the
> path much harder to audit.

OK.

> 
> > -
> > +#if __UAPI_DEF_IN6_ADDR
> >  struct in6_addr {
> >  	union {
> >  		__u8		u6_addr8[16];
> > +#if __UAPI_DEF_IN6_ADDR
> >  		__be16		u6_addr16[8];
> 
> The same CPP test twice inside of itself, the second test seems
> superfluous.  I bet this second one was supposed to be
> __UAPI_DEF_IN6_ADDR_ALT instead of __UAPI_DEF_IN6_ADDR.

Indeed.

> 
> > -#define IPPROTO_HOPOPTS		0	/* IPv6 hop-by-hop options	*/
> > -#define IPPROTO_ROUTING		43	/* IPv6 routing header		*/
> > -#define IPPROTO_FRAGMENT	44	/* IPv6 fragmentation header	*/
> > -#define IPPROTO_ICMPV6		58	/* ICMPv6			*/
> > -#define IPPROTO_NONE		59	/* IPv6 no next header		*/
> > -#define IPPROTO_DSTOPTS		60	/* IPv6 destination options	*/
> > -#define IPPROTO_MH		135	/* IPv6 mobility header		*/
> > +#if __UAPI_DEF_IPPROTO_V6
> > +enum {
> > +  IPPROTO_HOPOPTS = 0,		/* IPv6 hop-by-hop options      */
> 
> Again, do not reformat things, it's an unrelated change and makes
> this patch harder to review.

Ok.

I will update the patch.

Thanks!

  reply	other threads:[~2013-08-15  2:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-13  8:37 [Patch net-next] net: sync some IP headers with glibc Cong Wang
2013-08-13  8:37 ` [GLIBC Patch] inet: avoid redefinition of some structs in kernel Cong Wang
2013-08-14 20:42 ` [Patch net-next] net: sync some IP headers with glibc David Miller
2013-08-15  2:48   ` Cong Wang [this message]
2013-08-15  8:42   ` Cong Wang
2013-08-15  8:44     ` David Miller

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=1376534937.2626.6.camel@cr0 \
    --to=amwang@redhat.com \
    --cc=carlos@redhat.com \
    --cc=davem@davemloft.net \
    --cc=libc-alpha@sourceware.org \
    --cc=netdev@vger.kernel.org \
    --cc=tmb@mageia.org \
    --cc=yoshfuji@linux-ipv6.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.