Fix for dbi_initialize crash if libdbi >= 0.9.0 (notably on Ubuntu 14.04)

[Colin Law]

On 6 May 2014 13:09, John Ralls <jralls at ceridwen.us> wrote:
>
> On May 6, 2014, at 3:41 AM, Colin Law <clanlaw at gmail.com> wrote:
>
>> On 5 May 2014 23:18, John Ralls <jralls at ceridwen.us> wrote:
>>>
>>> On May 5, 2014, at 5:33 PM, Colin Law <clanlaw at gmail.com> wrote:
>>>
>>>> On 5 May 2014 21:45, John Ralls <jralls at ceridwen.us> wrote:
>>>>> ...
>>>>> It's because dbi_initialize_r() is the only function that needs to write to the dbi_instance; everything else just reads it. That makes managing the memory the application's job and allows other approaches than allocating it on the heap, including the static allocation that Moritz chose to use in his implementation. The "_r" suffix to the function name is analogous to gmtime_r, localtime_r, etc. which take a struct tm* from the caller instead of using a static in the library code. Making the rest of the functions use pass-by-copy saves a multi-threaded program from having to synchronize their calls, though using const dbi_inst* instead would have allowed for cleaner semantics when they choose to put it on the heap.
>>>>
>>>> That makes sense, though the fact that two users separately got the
>>>> interface wrong does make one wonder whether it is ideal.  It also
>>>> does not help that it is actually defined as void* or something
>>>> similar so the compiler was not able to identify that the original
>>>> call to initialize was wrong (I think, I have not looked at it in
>>>> detail).
>>>
>>> Of course it's not ideal, but it's the best one can do without proper constructors that are part of the language. In C++ one can simply have
>>>
>>>  static DbiInstance dbi_inst = DbiInstance();
>>>
>>> The compiler does more or less the same thing that dbi_initialize_r() does, but hides the gory details so that the programmer has to go out of his way to screw it up. As a consequence the compiler can be very strict about void*: The programmer has to explicitly declare or cast a parameter to void* in the function call or get a warning.
>>
>> The problem with the original code (which caused the runtime crash)
>> was that, because dbi_inst is defined as a void* the compiler was
>> unable to flag an error when a dbi_inst itself was passed as a
>> parameter instead of &dbi_inst.  It is a long time since I wrote any C
>> (as opposed to C++) and you are right that one forgets how much easier
>> it was to make such errors.  I wonder why dbi_inst is a void* rather
>> than a struct* as presumably it does point to a struct of some sort.
>
> I guess you're referring to
>   typedef void * dbi_inst
> in dbi.h. That's a clumsy way of making the pointer opaque. Unfortunately it completely subverts type safety, even in C++.
> Hmm. That also means that they've screwed up dbi_initialize_r(), because they still own the memory for the struct. They've
> accomplished absolutely nothing except breaking their API. What idiots.

The thing they *have* accomplished is the ability to have multiple
instances open at the same time, which I suspect was the prime motive.
 I agree, though, not the best way to achieve it.

Cheers

Colin