FAQ
Hello alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.


http://codereview.appspot.com/6506063/

Search Discussions

  • Alex Brainman at Sep 7, 2012 at 1:55 am
    Sorry, but I still fail to see the need for these changes. Perhaps, we
    could keep the test, but since I do not understand what it does, I can't
    say if it is good idea or not.

    Alex


    http://codereview.appspot.com/6506063/diff/8002/src/pkg/syscall/socket_windows_test.go
    File src/pkg/syscall/socket_windows_test.go (right):

    http://codereview.appspot.com/6506063/diff/8002/src/pkg/syscall/socket_windows_test.go#newcode46
    src/pkg/syscall/socket_windows_test.go:46: if err :=
    syscall.SetsockoptIPMreq(fd, syscall.IPPROTO_IP,
    syscall.IP_ADD_MEMBERSHIP, mreq); err != nil {
    s/fd/s/

    otherwise it fails to build

    # GOOS=windows go test -c
    # syscall_test
    ./socket_windows_test.go:46: undefined: fd

    http://codereview.appspot.com/6506063/diff/8002/src/pkg/syscall/syscall_windows.go
    File src/pkg/syscall/syscall_windows.go (right):

    http://codereview.appspot.com/6506063/diff/8002/src/pkg/syscall/syscall_windows.go#newcode712
    src/pkg/syscall/syscall_windows.go:712: func GetsockoptInt(fd Handle,
    level, opt int) (int, error) {
    This function is not used by go.net. Why do we need to implement it?

    http://codereview.appspot.com/6506063/diff/8002/src/pkg/syscall/syscall_windows.go#newcode729
    src/pkg/syscall/syscall_windows.go:729: func GetsockoptInet4Addr(fd
    Handle, level, opt int) ([4]byte, error) {
    This is a new function. You said that this CL simplify code in go.net,
    but looking there:

    # grep GetsockoptInet4Addr * -r

    ipv4/sockopt_bsd.go: a, err := syscall.GetsockoptInet4Addr(fd,
    syscall.IPPROTO_IP, syscall.IP_MULTICAST_IF)

    ipv4/sockopt_windows.go: a, err :=
    syscall.GetsockoptInet4Addr(fd, syscall.IPPROTO_IP,
    syscall.IP_MULTICAST_IF)

    #


    I can see it is called once from ipv4/sockopt_windows.go. You might as
    well implement it in go.net then.

    http://codereview.appspot.com/6506063/
  • Mikio Hara at Sep 7, 2012 at 4:01 am

    On Fri, Sep 7, 2012 at 10:55 AM, wrote:

    Sorry, but I still fail to see the need for these changes. Perhaps, we
    could keep the test, but since I do not understand what it does, I can't
    say if it is good idea or not.
    otherwise it fails to build
    oops, sorry.
    I can see it is called once from ipv4/sockopt_windows.go. You might as
    well implement it in go.net then.
    yup, looks like that's the point.

    I now plan to implement go.net/ipv4 and go.net/ipv6. those two will use
    Get/SetsockoptInt, SetsockoptIPMreq, IPv6Mreq and siblings.

    I think we have two options.

    a) syscall provides Setsockopt and Getsockopt
    other optional thin wrappers like GetsockoptInt, SetsockoptIPMreq,
    IPv6Mreq, blah blah should be implemented in each package

    b) syscall provides a bit detailed GetsockoptInt, SetsockoptIPMreq, etc

    (b) works well for Unix families; BSD bros, a cousin Darwin and a second
    cousin Liunx but I'm not sure for Windows families including 2000, XP, Vista
    and newcomers.

    Do you have any suggestions or ideas?
  • Alex Brainman at Sep 7, 2012 at 4:31 am

    On 2012/09/07 04:01:30, mikio wrote:

    Do you have any suggestions or ideas?
    Once something is in the main Go tree, it is hard to change it. I would
    keep everything in go.net.

    Alex

    http://codereview.appspot.com/6506063/
  • Mikioh Mikioh at Sep 7, 2012 at 6:04 am

    On 2012/09/07 04:31:12, brainman wrote:

    Once something is in the main Go tree, it is hard to change it. I
    would keep
    everything in http://go.net.
    ack, thanks!


    http://codereview.appspot.com/6506063/
  • Mikioh Mikioh at Sep 7, 2012 at 7:52 am

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedSep 6, '12 at 11:38a
activeSep 7, '12 at 7:52a
posts6
users2
websitegolang.org

2 users in discussion

Mikioh Mikioh: 4 posts Alex Brainman: 2 posts

People

Translate

site design / logo © 2022 Grokbase