Add getter methods for sockaddr.

Add getter methods for sockaddr.

Add methods to retrieve the address family, port, and address of the  
sockaddr.

Vinay Anantharaman <vinaya@adobe.com>;
BUG=N/A
TEST=Unit Tests
R=brettw, yzshen

[brettw, 2012-01-25 19:52:34]

LGTM

[commit-bot: I haz the power, 2012-01-25 19:53:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vinaya@adobe.com/9235035/1

[commit-bot: I haz the power, 2012-01-25 19:53:13]

Presubmit check for 9235035-1 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  ppapi/cpp/private/net_address_private.cc, line 37, 81 chars \
  ppapi/cpp/private/net_address_private.cc, line 83, 82 chars

vinaya@adobe.com is not in AUTHORS file. If you are a new contributor, please
visit
http://www.chromium.org/developers/contributing-code and read the "Legal"
section
If you are a chromite, verify the contributor signed the CLA.

Presubmit checks took 2.3s to calculate.

[viettrungluu, 2012-02-13 22:26:38]

Hrm, someone should probably fix this up and get it committed.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
File ppapi/shared_impl/private/net_address_private_impl.cc (right):

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:32: typedef ADDRESS_FAMILY
sa_family_t;
Probably this is no longer needed.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:55: return
reinterpret_cast<const sockaddr*>(addr->data)->sa_family;
Probably this should have a cast to uint16_t (from sa_family_t).

Or, moreover, we shouldn't "export" AF_INET/AF_INET6/whatever from the
platform's API to the Pepper API. (The values a) aren't standardized and b)
could even theoretically change depending on what headers+libraries are being
used to compile the browser.)

We should have the values in a Pepper header and translate from AF_* to PP_*.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:90: 
Extra blank line.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:352: if (GetFamily(&addr)
== AF_INET && addr.size >= sizeof(sockaddr_in))
It's not necessarily the case that AF_INET is a uint16_t.

[yzshen, 2012-02-13 22:30:58]

Hi, Trung.

(I should have updated this thread earlier.)

I have made some fixes and landed this patch:
http://codereview.chromium.org/9307115/

On Mon, Feb 13, 2012 at 2:26 PM, <viettrungluu@chromium.org> wrote:

> Hrm, someone should probably fix this up and get it committed.
>
>
> http://codereview.chromium.**org/9235035/diff/14005/ppapi/**
>
shared_impl/private/net_**address_private_impl.cc<http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/net_address_private_impl.cc>
> File ppapi/shared_impl/private/net_**address_private_impl.cc (right):
>
> http://codereview.chromium.**org/9235035/diff/14005/ppapi/**
>
shared_impl/private/net_**address_private_impl.cc#**newcode32<http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/net_address_private_impl.cc#newcode32>
> ppapi/shared_impl/private/net_**address_private_impl.cc:32: typedef
> ADDRESS_FAMILY sa_family_t;
> Probably this is no longer needed.
>
> http://codereview.chromium.**org/9235035/diff/14005/ppapi/**
>
shared_impl/private/net_**address_private_impl.cc#**newcode55<http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/net_address_private_impl.cc#newcode55>
> ppapi/shared_impl/private/net_**address_private_impl.cc:55: return
> reinterpret_cast<const sockaddr*>(addr->data)->sa_**family;
> Probably this should have a cast to uint16_t (from sa_family_t).
>
> Or, moreover, we shouldn't "export" AF_INET/AF_INET6/whatever from the
> platform's API to the Pepper API. (The values a) aren't standardized and
> b) could even theoretically change depending on what headers+libraries
> are being used to compile the browser.)
>
> We should have the values in a Pepper header and translate from AF_* to
> PP_*.
>
> http://codereview.chromium.**org/9235035/diff/14005/ppapi/**
>
shared_impl/private/net_**address_private_impl.cc#**newcode90<http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/net_address_private_impl.cc#newcode90>
> ppapi/shared_impl/private/net_**address_private_impl.cc:90:
> Extra blank line.
>
> http://codereview.chromium.**org/9235035/diff/14005/ppapi/**
>
shared_impl/private/net_**address_private_impl.cc#**newcode352<http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/net_address_private_impl.cc#newcode352>
> ppapi/shared_impl/private/net_**address_private_impl.cc:352: if
> (GetFamily(&addr) == AF_INET && addr.size >= sizeof(sockaddr_in))
> It's not necessarily the case that AF_INET is a uint16_t.
>
>
http://codereview.chromium.**org/9235035/<http://codereview.chromium.org/9235...
>



-- 
Best regards,
Yuzhu Shen.

[yzshen1, 2012-02-13 22:38:41]

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
File ppapi/shared_impl/private/net_address_private_impl.cc (right):

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:32: typedef ADDRESS_FAMILY
sa_family_t;
On 2012/02/13 22:26:39, viettrungluu wrote:
> Probably this is no longer needed.

I've fixed it before landing.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:55: return
reinterpret_cast<const sockaddr*>(addr->data)->sa_family;
On 2012/02/13 22:26:39, viettrungluu wrote:
> Probably this should have a cast to uint16_t (from sa_family_t).
> 
> Or, moreover, we shouldn't "export" AF_INET/AF_INET6/whatever from the
> platform's API to the Pepper API. (The values a) aren't standardized and b)
> could even theoretically change depending on what headers+libraries are being
> used to compile the browser.)
> 
> We should have the values in a Pepper header and translate from AF_* to PP_*.

This sounds good to me.

http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
ppapi/shared_impl/private/net_address_private_impl.cc:90: 
On 2012/02/13 22:26:39, viettrungluu wrote:
> Extra blank line.

I've fixed it before landing.

[viettrungluu, 2012-02-13 23:16:15]

On 2012/02/13 22:38:41, yzshen1 wrote:
>
http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
> File ppapi/shared_impl/private/net_address_private_impl.cc (right):
> 
>
http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
> ppapi/shared_impl/private/net_address_private_impl.cc:32: typedef
ADDRESS_FAMILY
> sa_family_t;
> On 2012/02/13 22:26:39, viettrungluu wrote:
> > Probably this is no longer needed.
> 
> I've fixed it before landing.
> 
>
http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
> ppapi/shared_impl/private/net_address_private_impl.cc:55: return
> reinterpret_cast<const sockaddr*>(addr->data)->sa_family;
> On 2012/02/13 22:26:39, viettrungluu wrote:
> > Probably this should have a cast to uint16_t (from sa_family_t).
> > 
> > Or, moreover, we shouldn't "export" AF_INET/AF_INET6/whatever from the
> > platform's API to the Pepper API. (The values a) aren't standardized and b)
> > could even theoretically change depending on what headers+libraries are
being
> > used to compile the browser.)
> > 
> > We should have the values in a Pepper header and translate from AF_* to
PP_*.
> 
> This sounds good to me.

Could you make this change? (Add PP_... corresponding to AF_INET and AF_INET6
(or whatever it is) to ppb_net_address_private.h.)

(You should check the values of AF_* on Linux/Mac/Windows, and if they're all
the same you should define the PP_... values correspondingly, and avoid changing
the version number of the interface.)

Thanks,

- Trung


> 
>
http://codereview.chromium.org/9235035/diff/14005/ppapi/shared_impl/private/n...
> ppapi/shared_impl/private/net_address_private_impl.cc:90: 
> On 2012/02/13 22:26:39, viettrungluu wrote:
> > Extra blank line.
> 
> I've fixed it before landing.

[yzshen1, 2012-02-13 23:22:44]

Sure. I will make the change.