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

[John Ralls]

On May 5, 2014, at 3:49 PM, Colin Law <clanlaw at gmail.com> wrote:

> On 5 May 2014 17:05, Geert Janssens <janssens-geert at telenet.be> wrote:
>> On Monday 05 May 2014 13:43:08 Colin Law wrote:
>> 
>>> Attached is patch to fix the startup crash on the maint branch after
>> 
>>> tag 2.6.3 when using libdbi >= 0.9.9, as is the case on Ubuntu 14.04.
>> 
>>> It is a single line correction (well single character in fact).
>> 
>>> 
>> 
>>> Do I need to file a bug and attach the patch there or can someone
>> 
>>> apply it from here?
>> 
>>> 
>> 
>>> Cheers
>> 
>>> 
>> 
>>> Colin
>> 
>> Hi Colin,
>> 
>> 
>> 
>> Thank you for the patch. Looking at the libdbi api documentation I believe
>> your fix is a step in the right direction. It only fixes the initialization
>> for sqlite files though and not the initialization for the other sql
>> backends.
>> 
>> 
>> 
>> So I have applied an updated patch for this.
>> 
>> 
>> 
>> In addition I have modified all dbi_<function>_r function calls to use
>> &dbi_instance instead of dbi_instance. That's how I understand it should be
>> from the api documentation. I have not tested this though because I don't
>> have libdbi 0.9.x on my system. Can you recheck with the most recent maint
>> revision and verify if everything works fine with your sqlite data file ?
> 
> Hi Geert
> 
> Could I ask you to have another look at the docs?  I think all the
> functions other than initialize actually take a dbi_inst rather than
> dbi_inst*.  Which is not the way I would have designed it certainly.
> I think the one call of dbi_initialize_r is for all db types.

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.

Regards,
John Ralls