From: Wang Chen <wangchen@cn.fujitsu.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, kaber@trash.net
Subject: Re: [PATCH net-next 1/8] af_packet: Check return of dev_set_promiscuity/allmulti
Date: Fri, 20 Jun 2008 10:13:18 +0800 [thread overview]
Message-ID: <485B123E.3020501@cn.fujitsu.com> (raw)
In-Reply-To: <20080619.190522.237900335.davem@davemloft.net>
David Miller said the following on 2008-6-20 10:05:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Fri, 20 Jun 2008 08:54:32 +0800
>
>> @@ -1245,7 +1247,8 @@ static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
>> i->count = 1;
>> i->next = po->mclist;
>> po->mclist = i;
>> - packet_dev_mc(dev, i, +1);
>> + /* Positive increment should be checked for overflow --WCN */
>> + err = packet_dev_mc(dev, i, 1);
>>
>
> Please don't add these little signatures to comments. That might have
> been useful to do 10 years ago when we didn't use proper source
> control, but now we do and anyone interested can do a "git blame"
> to see who added that comment and why.
>
> Also, this comment doesn't really add any information. We check
> error return values simply because errors can happen, that's just
> a straight fact. If packet_dev_mc() and it's sub calls can error
> for other reasons this comment is only telling part of the story
> and as a result becomes inaccurate.
>
> Therefore, I'd like to ask that you not add this comment, it doesn't
> really help anything. This kind of information can go into the
> commit log message. That's where "why" information tends to belong.
>
Roger.
I will remove all useless comment in my patch series.
Thanks David.
Don't blame me in every patch :)
next prev parent reply other threads:[~2008-06-20 2:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-20 0:54 [PATCH net-next 1/8] af_packet: Check return of dev_set_promiscuity/allmulti Wang Chen
2008-06-20 2:05 ` David Miller
2008-06-20 2:13 ` Wang Chen [this message]
2008-06-20 2:41 ` Wang Chen
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=485B123E.3020501@cn.fujitsu.com \
--to=wangchen@cn.fujitsu.com \
--cc=davem@davemloft.net \
--cc=kaber@trash.net \
--cc=netdev@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 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.