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. Regards, John Ralls _______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
