Change PPB_NetAddress_Private.GetFamily to return a PP_AddressFamily_Private value instead of uint1…

Change PPB_NetAddress_Private.GetFamily to return a PP_AddressFamily_Private value instead of uint16_t.

BUG=None
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=122228

[yzshen1, 2012-02-15 01:27:19]

Hi, Trung and Brett.

Would you please take a look? Thanks!

Trung: everything.
Bertt: owner stamp.

[brettw, 2012-02-15 01:32:08]

lgtm

[viettrungluu, 2012-02-15 01:43:03]

Obviously, efficiency concerns are a bit trivial, but it still bothers me a bit.

http://codereview.chromium.org/9398003/diff/1/ppapi/c/private/ppb_net_address...
File ppapi/c/private/ppb_net_address_private.h (right):

http://codereview.chromium.org/9398003/diff/1/ppapi/c/private/ppb_net_address...
ppapi/c/private/ppb_net_address_private.h:46: } PP_AddressFamily_Private;
I'm not sure I like this name a lot. (I might have called it
PP_NetAddress_Family_Private -- or is that PP_NetAddress_Private_Family?) (The
point being that it associates it with "NetAddress".) But whatever.

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
File ppapi/shared_impl/private/net_address_private_impl.cc (right):

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:63: if (GetFamily(addr) ==
PP_ADDRESSFAMILY_IPV4)  {
Now that GetFamily() has a nontrivial cost, you should probably use a switch
statement here. (That, or have a GetFamilyInternal() returns an sa_family_t ...
which would be ever-so-slightly more efficient.)

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:77: if (GetFamily(addr) ==
PP_ADDRESSFAMILY_IPV4)  {
"

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:100: if (GetFamily(addr1)
!= GetFamily(addr2))
Similarly, save the family of addr1 and compare it to GetFamily(addr2). (And use
a switch below.)

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:278: if
(GetFamily(src_addr) == PP_ADDRESSFAMILY_IPV4) {
"

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:354: if (GetFamily(&addr)
== PP_ADDRESSFAMILY_IPV4 &&
"

[yzshen1, 2012-02-15 18:40:55]

Thanks, Trung!
Please take another look.

http://codereview.chromium.org/9398003/diff/1/ppapi/c/private/ppb_net_address...
File ppapi/c/private/ppb_net_address_private.h (right):

http://codereview.chromium.org/9398003/diff/1/ppapi/c/private/ppb_net_address...
ppapi/c/private/ppb_net_address_private.h:46: } PP_AddressFamily_Private;
I did the change, but without the '_' between NetAddress and Family, so that it
is more consistent with others.

On 2012/02/15 01:43:03, viettrungluu wrote:
> I'm not sure I like this name a lot. (I might have called it
> PP_NetAddress_Family_Private -- or is that PP_NetAddress_Private_Family?) (The
> point being that it associates it with "NetAddress".) But whatever.

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
File ppapi/shared_impl/private/net_address_private_impl.cc (right):

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:63: if (GetFamily(addr) ==
PP_ADDRESSFAMILY_IPV4)  {
Done. (I did both.)

On 2012/02/15 01:43:03, viettrungluu wrote:
> Now that GetFamily() has a nontrivial cost, you should probably use a switch
> statement here. (That, or have a GetFamilyInternal() returns an sa_family_t
...
> which would be ever-so-slightly more efficient.)

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:77: if (GetFamily(addr) ==
PP_ADDRESSFAMILY_IPV4)  {
On 2012/02/15 01:43:03, viettrungluu wrote:
> "

Done.

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:100: if (GetFamily(addr1)
!= GetFamily(addr2))
On 2012/02/15 01:43:03, viettrungluu wrote:
> Similarly, save the family of addr1 and compare it to GetFamily(addr2). (And
use
> a switch below.)

Done.

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:278: if
(GetFamily(src_addr) == PP_ADDRESSFAMILY_IPV4) {
On 2012/02/15 01:43:03, viettrungluu wrote:
> "

Done.

http://codereview.chromium.org/9398003/diff/1/ppapi/shared_impl/private/net_a...
ppapi/shared_impl/private/net_address_private_impl.cc:354: if (GetFamily(&addr)
== PP_ADDRESSFAMILY_IPV4 &&
On 2012/02/15 01:43:03, viettrungluu wrote:
> "

Done.

[viettrungluu, 2012-02-16 01:38:03]

LGTM, thanks.

[commit-bot: I haz the power, 2012-02-16 01:42:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yzshen@chromium.org/9398003/5001

[commit-bot: I haz the power, 2012-02-16 03:09:46]

Change committed as 122228