Issue 8685045: code review 8685045: syscall: add missing IP-level socket option constants f... (Closed)
|
Created:
13 years, 2 months ago by mikio Modified:
13 years, 1 month ago CC:
golang-dev Visibility:
Public. |
Descriptionsyscall: add missing IP-level socket option constants for Windows
Update issue 1740
Patch Set 1 #Patch Set 2 : diff -r e729bb70416a https://code.google.com/p/go #Patch Set 3 : diff -r e729bb70416a https://code.google.com/p/go #
Total comments: 1
Patch Set 4 : diff -r e729bb70416a https://code.google.com/p/go #Patch Set 5 : diff -r 0c16e97c7587 https://code.google.com/p/go #MessagesTotal messages: 19
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
You're modifying a z- file? That's auto-generated and will just get blown away later, no? On Apr 24, 2013 4:40 AM, <mikioh.mikioh@gmail.com> wrote: > Reviewers: golang-dev1, > > Message: > Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > syscall: add missing IP constants for Windows > > Update issue 1740. > > Please review this at https://codereview.appspot.**com/8685045/<https://codereview.appspot.com/8685... > > Affected files: > M src/pkg/syscall/ztypes_**windows.go > > > Index: src/pkg/syscall/ztypes_**windows.go > ==============================**==============================**======= > --- a/src/pkg/syscall/ztypes_**windows.go > +++ b/src/pkg/syscall/ztypes_**windows.go > @@ -491,10 +491,19 @@ > SOCK_RAW = 3 > SOCK_SEQPACKET = 5 > > - IPPROTO_IP = 0 > - IPPROTO_IPV6 = 0x29 > - IPPROTO_TCP = 6 > - IPPROTO_UDP = 17 > + IPPROTO_AH = 0x33 > + IPPROTO_DSTOPTS = 0x3c > + IPPROTO_ESP = 0x32 > + IPPROTO_FRAGMENT = 0x2c > + IPOROTO_HOPOPTS = 0x0 > + IPPROTO_ICMP = 0x1 > + IPPROTO_ICMPV6 = 0x3a > + IPPROTO_IP = 0x0 > + IPPROTO_IPV6 = 0x29 > + IPPROTO_NONE = 0x3b > + IPPROTO_ROUTING = 0x2b > + IPPROTO_TCP = 0x6 > + IPPROTO_UDP = 0x11 > > SOL_SOCKET = 0xffff > SO_REUSEADDR = 4 > @@ -515,6 +524,7 @@ > > // cf. http://support.microsoft.com/**default.aspx?scid=kb;en-us;* > *257460 <http://support.microsoft.com/default.aspx?scid=kb;en-us;257460> > > + IP_HDRINCL = 0x2 > IP_TOS = 0x3 > IP_TTL = 0x4 > IP_MULTICAST_IF = 0x9 > @@ -522,14 +532,17 @@ > IP_MULTICAST_LOOP = 0xb > IP_ADD_MEMBERSHIP = 0xc > IP_DROP_MEMBERSHIP = 0xd > + IP_PKTINFO = 0x13 > > - IPV6_V6ONLY = 0x1b > IPV6_UNICAST_HOPS = 0x4 > IPV6_MULTICAST_IF = 0x9 > IPV6_MULTICAST_HOPS = 0xa > IPV6_MULTICAST_LOOP = 0xb > IPV6_JOIN_GROUP = 0xc > IPV6_LEAVE_GROUP = 0xd > + IPV6_PKTINFO = 0x13 > + IPV6_HOPLIMIT = 0x15 > + IPV6_V6ONLY = 0x1b > > SOMAXCONN = 0x7fffffff > > > > -- > > ---You received this message because you are subscribed to the Google > Groups "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou... > . > For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o... > . > > >
Sign in to reply to this message.
On Wed, Apr 24, 2013 at 11:19 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > You're modifying a z- file? Yup. > That's auto-generated and will just get blown away later, no? Unfortunately no z-file bootstrap stuff exists for both Windows and Plan 9.
Sign in to reply to this message.
please remove the period in the last line of the description, as the issue tracker only strictly recognize this format: Update issue NNNN cf: https://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_...
Sign in to reply to this message.
done, thx. On Thu, Apr 25, 2013 at 12:07 AM, <minux.ma@gmail.com> wrote: > please remove the period in the last line of the description, > as the issue tracker only strictly recognize this format: > Update issue NNNN > > cf: > https://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_... > > https://codereview.appspot.com/8685045/
Sign in to reply to this message.
But why to add all these constants? No one uses them. Alex
Sign in to reply to this message.
On Thu, Apr 25, 2013 at 5:58 AM, <alex.brainman@gmail.com> wrote: > But why to add all these constants? No one uses them. go.net/ipv4 and ipv6 will use them.
Sign in to reply to this message.
https://codereview.appspot.com/8685045/diff/3002/src/pkg/syscall/ztypes_windo... File src/pkg/syscall/ztypes_windows.go (right): https://codereview.appspot.com/8685045/diff/3002/src/pkg/syscall/ztypes_windo... src/pkg/syscall/ztypes_windows.go:498: IPOROTO_HOPOPTS = 0x0 typo. should be IPPROTO
Sign in to reply to this message.
With the typo fixed, I'd be fine with this. It does make all platforms consistent on including the full set. On Wed, Apr 24, 2013 at 2:58 PM, <bradfitz@golang.org> wrote: > > https://codereview.appspot.**com/8685045/diff/3002/src/pkg/** > syscall/ztypes_windows.go<https://codereview.appspot.com/8685045/diff/3002/src/pkg/syscall/ztypes_windows.go> > File src/pkg/syscall/ztypes_**windows.go (right): > > https://codereview.appspot.**com/8685045/diff/3002/src/pkg/** > syscall/ztypes_windows.go#**newcode498<https://codereview.appspot.com/8685045/diff/3002/src/pkg/syscall/ztypes_windows.go#newcode498> > src/pkg/syscall/ztypes_**windows.go:498: IPOROTO_HOPOPTS = 0x0 > typo. should be IPPROTO > > https://codereview.appspot.**com/8685045/<https://codereview.appspot.com/8685... >
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, bradfitz@golang.org, minux.ma@gmail.com, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
So I'd say this is fine, but why this set? Why not all of them? But really, these have nothing to do with system calls, right? Why not just define them in go.net somewhere? They seem totally portable. IPPROTO_3PC 0x22 IPPROTO_ADFS 0x44 IPPROTO_AH 0x33 IPPROTO_AHIP 0x3d IPPROTO_APES 0x63 IPPROTO_ARGUS 0xd IPPROTO_AX25 0x5d IPPROTO_BHA 0x31 IPPROTO_BLT 0x1e IPPROTO_BRSATMON 0x4c IPPROTO_CARP 0x70 IPPROTO_CFTP 0x3e IPPROTO_CHAOS 0x10 IPPROTO_CMTP 0x26 IPPROTO_COMP 0x6c IPPROTO_CPHB 0x49 IPPROTO_CPNX 0x48 IPPROTO_DCCP 0x21 IPPROTO_DDP 0x25 IPPROTO_DGP 0x56 IPPROTO_DIVERT 0x102 IPPROTO_DIVERT 0xfe IPPROTO_DONE 0x101 IPPROTO_DSTOPTS 0x3c IPPROTO_EGP 0x8 IPPROTO_EMCON 0xe IPPROTO_ENCAP 0x62 IPPROTO_EON 0x50 IPPROTO_ESP 0x32 IPPROTO_ETHERIP 0x61 IPPROTO_FRAGMENT 0x2c IPPROTO_GGP 0x3 IPPROTO_GMTP 0x64 IPPROTO_GRE 0x2f IPPROTO_HELLO 0x3f IPPROTO_HMP 0x14 IPPROTO_HOPOPTS 0x0 IPPROTO_ICMP 0x1 IPPROTO_ICMPV6 0x3a IPPROTO_IDP 0x16 IPPROTO_IDPR 0x23 IPPROTO_IDRP 0x2d IPPROTO_IGMP 0x2 IPPROTO_IGP 0x55 IPPROTO_IGRP 0x58 IPPROTO_IL 0x28 IPPROTO_INLSP 0x34 IPPROTO_INP 0x20 IPPROTO_IP 0x0 IPPROTO_IPCOMP 0x6c IPPROTO_IPCV 0x47 IPPROTO_IPEIP 0x5e IPPROTO_IPIP 0x4 IPPROTO_IPPC 0x43 IPPROTO_IPV4 0x4 IPPROTO_IPV6 0x29 IPPROTO_IPV6_ICMP 0x3a IPPROTO_IRTP 0x1c IPPROTO_KRYPTOLAN 0x41 IPPROTO_LARP 0x5b IPPROTO_LEAF1 0x19 IPPROTO_LEAF2 0x1a IPPROTO_MAX 0x100 IPPROTO_MAXID 0x103 IPPROTO_MAXID 0x34 IPPROTO_MEAS 0x13 IPPROTO_MH 0x87 IPPROTO_MHRP 0x30 IPPROTO_MICP 0x5f IPPROTO_MOBILE 0x37 IPPROTO_MPLS 0x89 IPPROTO_MTP 0x5c IPPROTO_MUX 0x12 IPPROTO_ND 0x4d IPPROTO_NHRP 0x36 IPPROTO_NONE 0x3b IPPROTO_NSP 0x1f IPPROTO_NVPII 0xb IPPROTO_OLD_DIVERT 0xfe IPPROTO_OSPFIGP 0x59 IPPROTO_PFSYNC 0xf0 IPPROTO_PGM 0x71 IPPROTO_PIGP 0x9 IPPROTO_PIM 0x67 IPPROTO_PRM 0x15 IPPROTO_PUP 0xc IPPROTO_PVP 0x4b IPPROTO_RAW 0xff IPPROTO_RCCMON 0xa IPPROTO_RDP 0x1b IPPROTO_ROUTING 0x2b IPPROTO_RSVP 0x2e IPPROTO_RVD 0x42 IPPROTO_SATEXPAK 0x40 IPPROTO_SATMON 0x45 IPPROTO_SCCSP 0x60 IPPROTO_SCTP 0x84 IPPROTO_SDRP 0x2a IPPROTO_SEND 0x103 IPPROTO_SEP 0x21 IPPROTO_SKIP 0x39 IPPROTO_SPACER 0x7fff IPPROTO_SRPC 0x5a IPPROTO_ST 0x7 IPPROTO_SVMTP 0x52 IPPROTO_SWIPE 0x35 IPPROTO_TCF 0x57 IPPROTO_TCP 0x6 IPPROTO_TLSP 0x38 IPPROTO_TP 0x1d IPPROTO_TPXX 0x27 IPPROTO_TRUNK1 0x17 IPPROTO_TRUNK2 0x18 IPPROTO_TTP 0x54 IPPROTO_UDP 0x11 IPPROTO_UDPLITE 0x88 IPPROTO_VINES 0x53 IPPROTO_VISA 0x46 IPPROTO_VMTP 0x51 IPPROTO_VRRP 0x70 IPPROTO_WBEXPAK 0x4f IPPROTO_WBMON 0x4e IPPROTO_WSN 0x4a IPPROTO_XNET 0xf IPPROTO_XTP 0x24 On Wed, Apr 24, 2013 at 5:59 PM, <alex.brainman@gmail.com> wrote: > LGTM > > https://codereview.appspot.**com/8685045/<https://codereview.appspot.com/8685... >
Sign in to reply to this message.
On 2013/04/25 02:30:23, bradfitz wrote: > So I'd say this is fine, but why this set? > Why not all of them? picked up what windows supports. > But really, these have nothing to do with system calls, right? they are parameters for socket syscalls (Socket, Setsockopt and Getsockopt) and ancillary data (once Windows supports Read/WriteMsg). > Why not just define them in http://go.net somewhere? They seem totally portable. IPPROTO_TCP from syscall and IPPROTO_XXX from somewhere else looks not good to me. No?
Sign in to reply to this message.
On Wed, Apr 24, 2013 at 7:44 PM, <mikioh.mikioh@gmail.com> wrote: > On 2013/04/25 02:30:23, bradfitz wrote: > > So I'd say this is fine, but why this set? >> Why not all of them? >> > > picked up what windows supports. > > > But really, these have nothing to do with system calls, right? >> > > they are parameters for socket syscalls (Socket, Setsockopt and > Getsockopt) and ancillary data (once Windows supports Read/WriteMsg). > > Why not just define them in http://go.net somewhere? They seem >> > totally portable. > > IPPROTO_TCP from syscall and IPPROTO_XXX from somewhere else > looks not good to me. No? > So put them all elsewhere. pkg syscall doesn't have to be clean and consistent. It's not even a goal.
Sign in to reply to this message.
On 2013/04/25 02:53:10, bradfitz wrote: > So put them all elsewhere. > > pkg syscall doesn't have to be clean and consistent. It's not even a goal. Does all you mean include IP_XXX, IPV6_XXX? Or only IANA stuff IPPROTOs?
Sign in to reply to this message.
Whatever stuff is defined by a standards body and not an OS. On Apr 24, 2013 8:39 PM, <mikioh.mikioh@gmail.com> wrote: > On 2013/04/25 02:53:10, bradfitz wrote: > > So put them all elsewhere. >> > > pkg syscall doesn't have to be clean and consistent. It's not even a >> > goal. > > Does all you mean include IP_XXX, IPV6_XXX? Or only IANA stuff IPPROTOs? > > https://codereview.appspot.**com/8685045/<https://codereview.appspot.com/8685... >
Sign in to reply to this message.
> Whatever stuff is defined by a standards body and not an OS. crystal clear, thanks.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, bradfitz@golang.org, minux.ma@gmail.com, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
|
