Hi Thibault
> _ The use of malloc may be prevented, by requesting a odbx_t* instead
> than odbx_t**. The allocation on the stack will thus reduce processor
> cache misses.
The reason why the memory for odbx_t is returned via odbx_t** is because
it has to be allocated dynamically anyway (there can be one or more
connections) and it's not returned by the function to be able to return
an error code in case anything goes wrong. I'm not sure how you would
like to do this on the stack.
> _ Putting the addresses of backend functions in a structure is risky
> if someone manages to overwrite it. Maybe just statically switching.
>
> _ Compiling the backends separately is a bit excessive, as they are
> not strictly backends since they use the client functions (seeing a
> backends folder I thought the pgsql plugin would talk the pgsql
> protocol ^^). To me the singlelib should be the only possibility
> (allowing the static switching from above).
>
> _ The use of void* fields is like unions, unrecommended. At the
> expense of a little more memory (storing all pgconn*, MYSQL*, ...
> handles side by side), you get static type checking.
These points are all based on your assumption that everybody only needs
a single .so file containing all "backends". Unfortunately, this isn't
true. The reason for separating the core library from the backends is
that they can be installed separately. Think about the Linux
distributions building .rpm or .deb files: There are dependencies
between the "backends" and the native database library. If you install
the OpenDBX library containing all backends as single library, the
distributions must install all native database libraries as well to
satisfy the dependencies. This is nothing the user or the packager would
like to to. The SINGLELIB option is mainly for embedded systems where
the requirements are exactly known and the memory/space is limited.
> _ There is no possibility to send the values separately from the
> statement (as with PQexecParams). I would argue it to be the only way
> to send values (and no explicit escaping).
>
> _ Sending values other than strings (just int, double and struct tm),
> and getting them also, like in dbi_result_get_fields from libdbi (but
> with less format bloat ^^).
Jep, you are right. The prepared statement API is on the todo list but I
haven't had time to implement this up to now.
> _ Why having two functions (init+bind) to connect? With pgsql it
> requires to store host and port strings. With a unique function the
> options could still be specified : if before connection, they will
> try to be matched when connecting, if after connection, the backend
> will try to update them. If these two functions are here to speed
> pooling, I think this feature will be rarely used in practice.
Different native database libraries have different requirements. What
you say is true for PostgreSQL, but not for all other DBMS libraries.
Furthermore, separating init() from bind() allows us to specify options
in a generic way and they are still available for rebinding.
> _ Casting out the majority of preprocessing instructions (see also
> debugging below).
>
> _ Use of NDEBUG, defined in assert.h, instead of ENABLE_DEBUGLOG,
> while logging in stderr and redirecting it to a file: #ifndef NDEBUG
> #define debug_printf(...) fprintf(stderr, __VA_ARGS__) #else #define
> debug_printf(...) ((void) 0) #endif freopen("log.txt", "w", stderr);
Good point.
> _ I like the current error codes - if negative you would have to
> manually set them, and to reorder if one becomes obsolete.
If one should ever become obsolete, it has to stay until the next major
release because otherwise we would break the ABI.
> _ Sometimes I see a chain of -> operators. In x86 each generates a
> LEA instruction, so I would argue to minimize their use as possible.
The OpenDBX library checks if the used pointer isn't NULL before
dereferencing it in order to prevent segfaults ... something you see
rather often in the MySQL library if you make a mistake ;-)
> BTW your code was really clean and easy to read!
Thanks a lot :-)
Norbert
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
libopendbx-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libopendbx-devel
http://www.linuxnetworks.de/doc/index.php/OpenDBX
