Simple TCP server socket code fails with '[SocketError]: Invalid argument' about rock

[danopia]

Is port 8080 open?

[nddrylliog]

I reproduced this bug on my machine and I don't think 8080 was open. Also, I have the same behavior with the stock test from nddrylliog/bootcamp and it's using another port (which, again, I'm not using at all).

[fredreichbier]

Same here.

[fredreichbier]

Hmm, "Invalid Argument" refers to the EINVAL errno. man 3 bind tells us:

       EINVAL The socket is already bound to an address, and the protocol does
              not support binding to a new address; or  the  socket  has  been
              shut down.

So, as danopia suggested, this error can be caused if the address is already bound, but I think we can eliminate this reason. So the socket is "shut down"? What does that mean?

[solson]

I get [SocketError]: Invalid argument in tsion/spry, too. (It's a client, not a server.)

[fredreichbier]

This is fixed in 1216b23 by using the cover class' size member for addr length() instead of the C operator sizeof. Take a look at the diff of the C output:

diff -ru faulty/custom-sdk/net/Address.c worky/custom-sdk/net/Address.c
--- faulty/custom-sdk/net/Address.c 2010-05-04 12:12:51.000000000 +0200
+++ worky/custom-sdk/net/Address.c  2010-05-04 12:13:04.000000000 +0200
@@ -757,7 +757,7 @@
 }
 
 lang_types__UInt32 net_Address__SocketAddressIP4_length_impl(net_Address__SocketAddressIP4* this) {
-    return ((lang_types__UInt32) (sizeof(net_berkeley__SockAddrIn_class())));
+    return ((lang_types__UInt32) (((lang_types__Class*) net_berkeley__SockAddrIn_class())->size));
 }
 
 void net_Address__SocketAddressIP4_init_SocketAddressIP4(net_Address__SocketAddressIP4* this, net_berkeley__InAddr addr, lang_types__Int port) {

sizeof is applied to the automatically-generated Class* of the SockAddrIn cover, thus returning an incorrect length (4 bytes on a 32 bit machine, because _class() returns a pointer), so an invalid length is passed to bind and we're getting the EINVAL.

This shows a daaangerous issue!

dramatic music

SomeValues: cover {
    a, b: Char
}

main: func {
    "SomeValues size = %d, sizeof(SomeValues) = %d" format(SomeValues size, sizeof(SomeValues)) println()
}

I'd expect SomeValue size to be equal to sizeof(SomeValue), but in fact the output is (for me on a 32 bit machine):

SomeValues size = 2, sizeof(SomeValues) = 4

sizeof(SomeType) gets translated to sizeof(SomeType_class()), which will always return the size of a void*, because sizeof always gets applied to the class pointer. I don't think this behaviour is wanted, though I find rock's behaviour completely consistent in this case: SomeType accesses the Class* of SomeType, and it's consistent that sizeof gets applied to the class pointer.

j/ooc did this differently, by the way:

SomeValues size = 2, sizeof(SomeValues) = 2

However, I'd tend to keep rock's behaviour here. Defining an exception to our lovely "X is the class" rule doesn't sound like a good idea to me.

So IMHO, we should avoid using sizeof and use class size instead. A quick ack shows that in 1216b23, our sdk uses sizeof in some places:

custom-sdk/text/Regexp.ooc
77:        ovector: Int* = gc_malloc(sizeof(Int) * maxSubstrings)

custom-sdk/net/StreamSocket.ooc
51:        if(socket send(descriptor, byte&, sizeof(Char), flags) == -1) {
68:        if(socket recv(descriptor, c&, sizeof(Char), flags) == -1) {

custom-sdk/net/Address.ooc
164:        memcpy(ai&, addr&, sizeof(InAddr))
225:        memcpy(ai&, addr&, sizeof(In6Addr))
320:        if(len == sizeof(SockAddrIn)) {
323:        else if(len == sizeof(SockAddrIn6)) {
348:        memset(sa&, 0, sizeof(SockAddrIn))
350:        memcpy(sa sin_addr&, addr&, sizeof(InAddr))
354:        memcpy(sa&, sockAddr, sizeof(SockAddrIn))
369:        memset(sa&, 0, sizeof(SockAddrIn6))
371:        memcpy(sa sin6_addr&, addr&, sizeof(In6Addr))
375:        memcpy(sa&, sockAddr, sizeof(SockAddrIn6))
383:    length: func -> UInt32 { sizeof(SockAddrIn6) }

custom-sdk/net/DNS.ooc
15:        memset(hints&, 0, sizeof(hints))

custom-sdk/internals/yajit/BinarySeq.ooc
18:        memcpy(data, d, size * sizeof(UChar))
22:        memsize := size * sizeof(UChar)

For now, I've replaced all occurences of sizeof with class size in 5d2e460.

So what do you think about this? :-)

[nddrylliog]

Well I mostly agree. Although then - is sizeof really any use at all? Shouldn't we just remove its extern declaration in the sdk?

[fredreichbier]

Well, there is no equivalent to sizeof(int*) for eaxmple, but I don't think that's really needed anyway. So, +1 for removal

[nddrylliog]

Well, if I'm not mistaken, all pointers have the same size (except in C++ for some method pointers - but nobody really agrees on that).

sizeof will self-destruct in 10...

[nddrylliog]

Okay so if you want sizeof(int*), 'Pointer size' should work for you.

Anyway, closed by 9b59721

[pheuter]

Nice