On 6 May 2014 13:09, John Ralls <[email protected]> wrote: > > On May 6, 2014, at 3:41 AM, Colin Law <[email protected]> wrote: > >> On 5 May 2014 23:18, John Ralls <[email protected]> wrote: >>> >>> On May 5, 2014, at 5:33 PM, Colin Law <[email protected]> wrote: >>> >>>> On 5 May 2014 21:45, John Ralls <[email protected]> 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 _______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
