|
From: Stephen S. <rad...@gm...> - 2013-11-27 14:02:03
|
On Wed, Nov 27, 2013 at 2:20 PM, Stephen Sinclair <rad...@gm...> wrote:
> On Wed, Nov 27, 2013 at 2:05 PM, Stephen Sinclair <rad...@gm...> wrote:
>> 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.
>
> I'll just note that I'm experimenting with changing the typedefs in
> lo_types.h from void* to struct{}*, and it's exposing a bunch more
> similar mistakes.
>
> I've always found the argument ordering in lo_send_message_from() to
> be confusing. :(
>
> For some reason I always expect these functions to be called
> lo_message_send() and lo_message_send_from(), and take a lo_message as
> first argument.. I get it wrong almost every time. We should
> definitely try to get the compiler to flag these kind of mistakes, (if
> it's possible without breaking everyone's code.)
I pushed a proposed change to a github branch called "struct_types":
https://github.com/radarsat1/liblo/tree/struct_types
I think this is a positive change, but it could have implications for
user code since technically it is an API change, although I believe it
does not change the ABI.
I'm of two minds on whether to merge this. Pros:
- It better-ensures correct API usage.
- It provides enough information that it fixes the bad implicit cast
bug for lo::ServerThread in cpp_test.c.
Cons:
- Current user code that sloppily uses void* in place of some types
will now fail to compile. This includes some of the liblo examples!
Not sure whether to push it or not.. perhaps the bug fixes could be
pushed in 0.28 and API changes left to the next version. (Perhaps
it's time to think about bumping the major version.)
On the other hand I hesitate to leave the implicit cast problem in the
C++ wrapper, although that is totally new code and shouldn't affect
existing code bases.
Steve
|