Re: [liblo] getting reading for 0.28-rc

On Tue, Nov 26, 2013 at 5:51 PM, Camille Troillard
<ca...@os...> wrote:
> Hi Steve,
>
> A little story ...
>
> When running cpp_test built for i386.
> If I comment the cpp_test.cpp:152, no error occurs.
>
>     a.send_from(st, "test1", "i", 20);
>
> More precisely, the same happens if lo_cpp.h:138 is commented:
>
>     int r = lo_send_message_from(from, address, path, m);
>
>
> Running cpp_test in lldb shows that the error occurs when lo_server_free is called:
>
> frame #4: 0x0007eede liblo.7.dylib`lo_server_free(s=0x0014f720) + 350 at server.c:726
>    723              s->ai = NULL;
>    724          }
>    725          if (s->hostname) {
> -> 726              free(s->hostname);
>    727              s->hostname = NULL;
>    728          }
>    729          if (s->path) {
>
> At this point s->hostname == 0x9
>
> I stepped around the call at lo_cpp.h:138 and found that the hostname field of the lo_server was as expected “” (empty string) before the call as expected, then became 0x1 when entered in the body of lo_send_message_from!
>
> Still stepping, at send.c:541, I can see “a->errnum = geterror()“ where a is a lo_address and errnum == 0x9.  It became clear that there was a problem in the function call.  Going back to lo_cpp.h showed that lo_send_message_from was called with the first two arguments swapped.
>
> “That was easy”, but took me two hours!  How can we avoid that?  I believe for some reason the warnings are muffled when building from the Makefile.  It would be nice to enable them, but I don’t know how to fix that.

Sorry it wasn't an obvious one :-/   I agree that there should have
been a warning for this kind of thing, but I don't think warnings were
actually muffled here.  In fact if you use debug mode, the command
line has -Wall and -Werror turned on.

Rather, I think it's because lo_types.h has both lo_server and
lo_address typedef'd to void*, which the compiler treats as
interchangeable.  So technically there is no warning to issue if these
types get mixed up.

There could be ways to fix this using typedef'd structs instead of
void pointers or something like that... I'm not sure whether such a
change wouldn't have pretty bad implications for the API though.  I'll
look into it.

>
> After this fix another bug arose, it looks like data corruption too, but haven’t found out why.

I see it.  Actually I know what's happening here.. it's a weird
interaction between C++ inheritance, implicit casting, and the above
problem.

In my wrapper, the lo::ServerThread class inherits from lo::Server.
They both autocast to their respective lo_serverthread and lo_server
types.  However, I had intended that if you pass a lo::ServerThread to
a function expecting lo_server, it would call the superclass's
lo_server() operator.

Unfortunately this doesn't seem to happen.. instead of upcasting to
lo::Server and calling lo::Server::lo_server(), it silently calls
lo::ServerThread::lo_serverthread() and passes in a lo_serverthread to
lo_message_send_from, where a lo_server is expected.

This is, I presume, rooted in the whole problem of void* typedefs
being considered equal in C, if I understand correctly.  The C++
compiler doesn't see the need to pass in a lo_server rather than
lo_serverthread, and therefore doesn't cast correctly.

You can fix the bug by changing the line,

a.send_from(st, "test1", "i", 20);

to:

a.send_from((lo::Server&)st, "test1", "i", 20);

Weirdly, if you don't use a reference it doesn't compile:

a.send_from((lo::Server)st, "test1", "i", 20);

apparently in this case it is trying to call a non-existing copy
constructor, I'm not sure why.

Anyways, I'm just looking for a better solution, trying to get the
implicit cast to work as I intended, but I haven't got it figured out
just yet.  At worst case overloading could be used to fix it perhaps.

Steve