liblo-devel Mailing List for LibLO

On Sat, Apr 28, 2012 at 10:54 AM, Camille Troillard
<ca...@os...> wrote:
> Dear liblo-developers,
>
> I often found myself confused by the usefulness of the LO_DEFAULT macro.
>
> On Mac OS X, when a lo_server is created with the protocol LO_DEFAULT, the OSC server listens on a UDP socket, which has the same effect as using LO_UDP as protocol. But when yo compare two protocol values, since they are integers you would want to do : proto1 == proto2, but you can't because LO_DEFAULT and LO_UDP are the same semantically, but don't have the same value.  Before I realize that, this problem has introduced some bugs in my code.
>
> Now I am using a custom function to compare protocols together, but it feels like a hack to me.
>
> IMHO, I would suggest to get rid of the macro LO_DEFAULT, and use LO_UDP where a default value is needed.

I guess the reason it exists is because LO_DEFAULT corresponds to
LO_UNIX if a port value is given that starts with '/'.  Not sure how
useful that is in practice.  But in any case, lo_server_new() replaces
the protocol identifier with LO_UDP or LO_UNIX depending on this.  So
comparing the output of lo_server_get_protocol() should work.  I'm
curious in what context you're involving LO_DEFAULT here.  Are you
making some comparison before initializing a server?

Steve

On Sat, Apr 28, 2012 at 9:42 AM, Camille Troillard
<ca...@os...> wrote:
> Hello,
>
> In message.c at line 942, lo_arg_pp_internal is called with the last argument (bigendian) set to 0.
> This causes values to print incorrectly on little endian cpu based computers.

That line is:

lo_arg_pp_internal(m->types[i], d, 1);

Do you mean "set to 1"?


> I have not fully understood how endianness is detected within liblo, so I fear I don't have a portable solution to give, but I am sure it is easy to fix.

My understanding is that lo_arg_pp needs to be told whether the data
has been converted from network order or not.  So lo_arg_pp() calls it
with bigendian=0, since the data has presumably already been
deserialised.  Whereas lo_message_pp calls it with bigendian=1, since
inside the lo_message it points to serialised data?

However, this seems to be not true, as you point out.  Data pointed to
by lo_message is always already converted to native order, so it
should always be called with bigendian=0 even in lo_message_pp.
Confusing. It would seem only useful when referring to serialised
data.  I guess I'll need to test on little and big endian machines to
be sure.  Fortunately I still have access to a ppc mac.

Steve

Dear liblo-developers,

I often found myself confused by the usefulness of the LO_DEFAULT macro.

On Mac OS X, when a lo_server is created with the protocol LO_DEFAULT, the OSC server listens on a UDP socket, which has the same effect as using LO_UDP as protocol. But when yo compare two protocol values, since they are integers you would want to do : proto1 == proto2, but you can't because LO_DEFAULT and LO_UDP are the same semantically, but don't have the same value.  Before I realize that, this problem has introduced some bugs in my code.

Now I am using a custom function to compare protocols together, but it feels like a hack to me.

IMHO, I would suggest to get rid of the macro LO_DEFAULT, and use LO_UDP where a default value is needed.



Best,
Camille

Hello,

In message.c at line 942, lo_arg_pp_internal is called with the last argument (bigendian) set to 0.
This causes values to print incorrectly on little endian cpu based computers.

I have not fully understood how endianness is detected within liblo, so I fear I don't have a portable solution to give, but I am sure it is easy to fix.



Best,
Cam

Hi Stephen,


On 9 avr. 2012, at 22:25, Stephen Sinclair wrote:

>> I think a proper solution would be to maintain the state read_size and bytesRead at the level of the lo_server (like a small state machine) and adequately read the message with the available data, or wait for more data.  If not all the data has been read, then we wait again for more data, and continue the reading were it was left.
>> 
>> Stephen, what do you think of this solution?  I think this would pose a semantic problem with lo_wait and lo_server_read because in fact you don't read a message as it fails in the middle of reading it.
> 
> I see what you mean, it would violate the idea that after lo_wait()
> there should be a full message read before returning.

Yes, in fact I was thinking about a previous discussion where you said that semantically lo_server_recv was supposed to return something, but that's the same idea.


> That's actually
> quite tough, I will have to play with it a little to see if there is a
> solution.  Just acknowledging your bug report for now.

Thank you.
I can live with the temporary fix I have implemented.


> I wonder if it means this semantic aspect of lo_server_recv() is not feasible in the
> general case.

If you think about the semantics of the recv call, it does not guarantee you that you will receive exactly the amount of bytes you requested (hence our discussion!), so it wouldn't shock me that lo_server_recv behaves the same as recv but at the application level, i.e. you are not guaranteed to receive the exact number of OSC messages you expected (one message for instance).


> Unless lo_wait() actually reads the message and queues
> it up, leaving lo_server_recv() to perform dispatching.. maybe that's
> not a terrible idea?

Sounds like a nice idea.

What about queuing the received bytes instead of queuing the message, or maybe that's what you meant?  Just keep in mind that even the byte count prefix could be truncated, which means that you don't even have a message at this level.  In the end what is important is to make sure you have read 4 bytes for the size, then N bytes to decode the complete OSC message.


Best Regards,
Cam

On Fri, Apr 6, 2012 at 12:01 PM, Camille Troillard
<ca...@os...> wrote:
> I think a proper solution would be to maintain the state read_size and bytesRead at the level of the lo_server (like a small state machine) and adequately read the message with the available data, or wait for more data.  If not all the data has been read, then we wait again for more data, and continue the reading were it was left.
>
> Stephen, what do you think of this solution?  I think this would pose a semantic problem with lo_wait and lo_server_read because in fact you don't read a message as it fails in the middle of reading it.


I see what you mean, it would violate the idea that after lo_wait()
there should be a full message read before returning.  That's actually
quite tough, I will have to play with it a little to see if there is a
solution.  Just acknowledging your bug report for now.  I wonder if it
means this semantic aspect of lo_server_recv() is not feasible in the
general case.  Unless lo_wait() actually reads the message and queues
it up, leaving lo_server_recv() to perform dispatching.. maybe that's
not a terrible idea?

Steve

On Fri, Apr 6, 2012 at 12:06 PM, Camille Troillard
<ca...@os...> wrote:
> Hello again,
>
>
> I have a liblo server configured with TCP and a client (not liblo) that communicated with my server.  When the client socket is closed, the lo_wait function returns and call to lo_server_recv throws an error because 0 bytes are read.  In this case, reading 0 bytes is not an error (on Mac OS X at least), but rather a sign that the other party properly closed and shutdown its socket.
>
> See this extract from man recv(2) on Mac OS X:
>
>  "If no messages are available to be received and the peer has performed an orderly shutdown, the value 0 is returned."
>
>
> Which means that lo_server_recv_raw_stream (server.c:773) doesn't need to throw an error when the receive data is 0.
>

Uh yeah I agree that should be an expected behaviour ;) That's the
normal way to signify a closed connection.
Would be great to write a little TCP-specific test suite maybe...


thanks,
Steve

I'd like to mention this thread on Stack Overflow that is related to the problem I described:
http://stackoverflow.com/questions/1837444/tcp-connection-seems-to-receive-incomplete-data


Please note that the send function in send.c suffer from the same bug as in server.c.



On 6 avr. 2012, at 18:01, Camille Troillard wrote:

> Dear liblo list,
> 
> When a liblo server is used with the TCP protocol on a very unreliable network (e.g. weak wireless connectivity), the lo_server_recv_raw_stream function does not work as expected, more specifically the code at line 767 in server.c:
> 
>            ret = recv(sock, &read_size, sizeof(read_size), 0);
>            read_size = ntohl(read_size);
>            if (read_size > LO_MAX_MSG_SIZE || ret <= 0) {
>                closesocket(sock);
>                lo_server_del_socket(s, i, sock);
>                if (ret > 0)
>                    lo_throw(s, LO_TOOBIG, "Message too large", "recv()");
>                continue;
>            }
>            ret = recv(sock, buffer, read_size, 0);
>            if (ret <= 0) {
>                closesocket(sock);
>                lo_server_del_socket(s, i, sock);
>                continue;
>            }
> 
> 
> In the previous code, there are two spots for a bug because there is no check for the returned length of 'ret'.
> The first bug will occur of less than 4 bytes are read, then the second bug will occur if there is less than read_size bytes read.
> 
> To simulate an unreliable network, I just used the old wireless access point that I use at home, or with a good access point, I installed the preference pane "SPEED LIMIT" on my MacBook Pro and set the delay to 2500 ms.
> 
> 
> Here is a fix that fixes the bug during my tests, but I am not happy with the solution (I will explain later):
> 
>            ret = recv(sock, &read_size, sizeof(read_size), 0);
>            read_size = ntohl(read_size);
>            if (read_size > LO_MAX_MSG_SIZE || ret != sizeof(read_size)) {
>                closesocket(sock);
>                lo_server_del_socket(s, i, sock);
>                if (ret != sizeof(read_size))
>                    lo_throw(s, LO_TOOBIG, "Message too large or read failed", "recv()");
>                continue;
>            }
> 			
>            int bytesRead = 0;
> 
>            do
>            {
>                ret = recv(sock, &buffer[bytesRead], read_size - bytesRead, 0);
>                if (ret <= 0)
>                    break;
> 				
>                bytesRead += ret;
>            }
>            while (bytesRead < read_size);
> 				
>            if (ret <= 0) {
>                closesocket(sock);
>                lo_server_del_socket(s, i, sock);
>                continue;
>            }
> 			
>           ret = bytesRead;
> 
> 
> This solution is not good, it only outlines the problem and give a temporary fix.  It does not handle the situation where less than 4 bytes are read in the first call to recv, and when used in the context of a non-blocking call (e.g. preceded by lo_wait, and then a call to lo_server_recv), this code does not work as desired because it blocks the current thread for an indefinite amount of time.
> 
> I think a proper solution would be to maintain the state read_size and bytesRead at the level of the lo_server (like a small state machine) and adequately read the message with the available data, or wait for more data.  If not all the data has been read, then we wait again for more data, and continue the reading were it was left.
> 
> Stephen, what do you think of this solution?  I think this would pose a semantic problem with lo_wait and lo_server_read because in fact you don't read a message as it fails in the middle of reading it.
> 
> Any thoughts?
> 
> 
> Best Regards,
> Cam
> 

Hello again,


I have a liblo server configured with TCP and a client (not liblo) that communicated with my server.  When the client socket is closed, the lo_wait function returns and call to lo_server_recv throws an error because 0 bytes are read.  In this case, reading 0 bytes is not an error (on Mac OS X at least), but rather a sign that the other party properly closed and shutdown its socket.

See this extract from man recv(2) on Mac OS X:

  "If no messages are available to be received and the peer has performed an orderly shutdown, the value 0 is returned."


Which means that lo_server_recv_raw_stream (server.c:773) doesn't need to throw an error when the receive data is 0.


Best Regards,
Cam

Dear liblo list,

When a liblo server is used with the TCP protocol on a very unreliable network (e.g. weak wireless connectivity), the lo_server_recv_raw_stream function does not work as expected, more specifically the code at line 767 in server.c:

            ret = recv(sock, &read_size, sizeof(read_size), 0);
            read_size = ntohl(read_size);
            if (read_size > LO_MAX_MSG_SIZE || ret <= 0) {
                closesocket(sock);
                lo_server_del_socket(s, i, sock);
                if (ret > 0)
                    lo_throw(s, LO_TOOBIG, "Message too large", "recv()");
                continue;
            }
            ret = recv(sock, buffer, read_size, 0);
            if (ret <= 0) {
                closesocket(sock);
                lo_server_del_socket(s, i, sock);
                continue;
            }


In the previous code, there are two spots for a bug because there is no check for the returned length of 'ret'.
The first bug will occur of less than 4 bytes are read, then the second bug will occur if there is less than read_size bytes read.

To simulate an unreliable network, I just used the old wireless access point that I use at home, or with a good access point, I installed the preference pane "SPEED LIMIT" on my MacBook Pro and set the delay to 2500 ms.


Here is a fix that fixes the bug during my tests, but I am not happy with the solution (I will explain later):

            ret = recv(sock, &read_size, sizeof(read_size), 0);
            read_size = ntohl(read_size);
            if (read_size > LO_MAX_MSG_SIZE || ret != sizeof(read_size)) {
                closesocket(sock);
                lo_server_del_socket(s, i, sock);
                if (ret != sizeof(read_size))
                    lo_throw(s, LO_TOOBIG, "Message too large or read failed", "recv()");
                continue;
            }
			
            int bytesRead = 0;

            do
            {
                ret = recv(sock, &buffer[bytesRead], read_size - bytesRead, 0);
                if (ret <= 0)
                    break;
				
                bytesRead += ret;
            }
            while (bytesRead < read_size);
				
            if (ret <= 0) {
                closesocket(sock);
                lo_server_del_socket(s, i, sock);
                continue;
            }
			
           ret = bytesRead;


This solution is not good, it only outlines the problem and give a temporary fix.  It does not handle the situation where less than 4 bytes are read in the first call to recv, and when used in the context of a non-blocking call (e.g. preceded by lo_wait, and then a call to lo_server_recv), this code does not work as desired because it blocks the current thread for an indefinite amount of time.

I think a proper solution would be to maintain the state read_size and bytesRead at the level of the lo_server (like a small state machine) and adequately read the message with the available data, or wait for more data.  If not all the data has been read, then we wait again for more data, and continue the reading were it was left.

Stephen, what do you think of this solution?  I think this would pose a semantic problem with lo_wait and lo_server_read because in fact you don't read a message as it fails in the middle of reading it.

Any thoughts?


Best Regards,
Cam