mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Darya Litvintseva <litv.daria@gmail.com>
Cc: alex.mikhalevich@gmail.com,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: fix braces and comments codestyle
Date: Mon, 10 Dec 2018 15:23:18 -0800	[thread overview]
Message-ID: <128afaebcd8fdb52bd90dcc9a09ee2d23d306a5a.camel@perches.com> (raw)
In-Reply-To: <20181210231140.5280-1-litv.daria@gmail.com>

On Tue, 2018-12-11 at 02:11 +0300, Darya Litvintseva wrote:
> Signed-off-by: Darya Litvintseva <litv.daria@gmail.com>

Hello.

Most maintainers want some patch description
and not a blank commit message.

Whitespace only changes are sometimes not taken.

First kernel patches are best done in drivers/staging.

And some additional comments below...

cheers, Joe

> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
[]
> @@ -90,9 +90,7 @@ int eth_header(struct sk_buff *skb, struct net_device *dev,
>  	else
>  		eth->h_proto = htons(len);
>  
> -	/*
> -	 *      Set the source hardware address.
> -	 */
> +	/* Set the source hardware address.*/

Most single line comments use a space before the */

> @@ -170,13 +166,11 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  			skb->pkt_type = PACKET_BROADCAST;
>  		else
>  			skb->pkt_type = PACKET_MULTICAST;
> -	}
> -	else if (unlikely(!ether_addr_equal_64bits(eth->h_dest,
> +	} else if (unlikely(!ether_addr_equal_64bits(eth->h_dest,
>  						   dev->dev_addr)))

please realign multiline statements to the open parenthesis

> @@ -187,8 +181,7 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  	if (likely(eth_proto_is_802_3(eth->h_proto)))
>  		return eth->h_proto;
>  
> -	/*
> -	 *      This is a magic hack to spot IPX packets. Older Novell breaks
> +	/*      This is a magic hack to spot IPX packets. Older Novell breaks
>  	 *      the protocol design and runs IPX over 802.3 without an 802.2 LLC
>  	 *      layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
>  	 *      won't work for fault tolerant netware but does for the rest.

Given the ipx protocol removal, perhaps this block
should be revisited.

commit e02554e9a4338c58e75fdfb0ef908a5adc86cba5
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Tue Nov 14 08:37:14 2017 -0800

    ipx: move Novell IPX protocol support into staging

commit 7a2e838d28cff6718a0bdf66164465402f8e40ed
Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date:   Fri Jun 1 20:39:54 2018 +0200

    staging: ipx: delete it from the tree
    
    The ipx code moved into the staging tree back in November 2017 and no
    one has complained or even noticed it was gone.  Because of that, let's
    just delete it.

> @@ -197,8 +190,7 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  	if (sap && *sap == 0xFFFF)
>  		return htons(ETH_P_802_3);
>  
> -	/*
> -	 *      Real 802.2 LLC
> +	/* Real 802.2 LLC
>  	 */

better to move the closing */ up as a single line comment



      reply	other threads:[~2018-12-10 23:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10 23:11 Darya Litvintseva
2018-12-10 23:23 ` Joe Perches [this message]

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=128afaebcd8fdb52bd90dcc9a09ee2d23d306a5a.camel@perches.com \
    --to=joe@perches.com \
    --cc=alex.mikhalevich@gmail.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=litv.daria@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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

all inboxes | Powered by JetHome®