From: Yang Hongyang <yanghy@cn.fujitsu.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, yoshfuji@linux-ipv6.org
Subject: Re: [PATCH]ipv6:use ipv6_addr_type instead of __ipv6_addr_type
Date: Fri, 08 May 2009 16:13:33 +0800 [thread overview]
Message-ID: <4A03E9AD.3080105@cn.fujitsu.com> (raw)
In-Reply-To: <20090508.004420.22402966.davem@davemloft.net>
David Miller wrote:
> From: Yang Hongyang <yanghy@cn.fujitsu.com>
> Date: Fri, 08 May 2009 09:18:11 +0800
>
>> David Miller wrote:
>>> From: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> Date: Thu, 07 May 2009 15:47:38 +0800
>>>
>>>> Although the function of these two functions are the same,we should use the
>>>> encapsulation function of __ipv6_addr_type
>>>>
>>>> Signed-off-by: Yang Hongyang<yanghy@cn.fujitsu.com>
>>> Are you sure? ipv6_addr_type() masks out the upper 16-bits
>>> of __ipv6_addr_type()'s return value, which is the scope.
>>>
>>> I'm not convinced that is what we want to do here.
>> My advice here is that the function which has __ maybe shouldn't be directly used.
>
> You're using vague terms like "advice" and "maybe" and this
> only decreases my confidence in your change.
Yes,I said maybe because I judge this change by search the code.There are many
places that use ipv6_addr_type() but only three places out of ipv6.h use
__ipv6_addr_type().Now, there are two methods can to do the right things,
one is ipv6_addr_type(),another is __ipv6_addr_type().There's no doult that
These three places can be converted to use ipv6_addr_type().I said converted
because it's not just change __ipv6_addr_type() to ipv6_addr_type().
Of course you can say that this dose not imply that we *should* do this change.
I agree,so I just said this is a advice:)
>
> You're also not addressing my questions at all.
>
> Two "_" characters at the beginning of the function's name says
> nothing about whether these call sites want the scope bits present in
> the return value they receive.
I used to think Two "_" means the funtion shouldn't(or should better not)
be called outside of the file.You teach me,thank you!
>
> This seems like a completely pointless and mindless change,
> and in fact might introduce bugs.
If you think this is a pointless change,you can just ingore it,
So sorry for the inconvenience:(But I don't think it's mindless,Because
before you teach me, I really misunderstand Two "_" means .
>
> There is no way I'm applying this. You can't even explain if the
> patch is correct or not. In fact, I doubt you even know.
>
Yes,this single patch is not correct.As I said,this patch should together with
another patch to covert use of __ipv6_addr_src_scope to ipv6_addr_src_scope.
--
Regards
Yang Hongyang
prev parent reply other threads:[~2009-05-08 8:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-07 7:47 [PATCH]ipv6:use ipv6_addr_type instead of __ipv6_addr_type Yang Hongyang
2009-05-07 21:59 ` David Miller
2009-05-08 1:18 ` Yang Hongyang
2009-05-08 7:44 ` David Miller
2009-05-08 7:48 ` David Miller
2009-05-08 8:18 ` Yang Hongyang
2009-05-08 8:13 ` Yang Hongyang [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=4A03E9AD.3080105@cn.fujitsu.com \
--to=yanghy@cn.fujitsu.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.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.