[OpenSER-Devel] database module issues (rant)

[Henning Westerholt]

Hi all,

some thoughts about issues that i've encountered during my journey through the 
database modules. Sorry about this rant, but it's a little bit tiring to 
going through this issues again and again. So if you think about writing a 
new database module, here is a list with some points to check against.. ;-)


1. Error checking

I think its absolut necessary to check for the validity of your input 
parameter. Otherwise you'll crash on the first null pointer that you get. 
The job of the database modules is to provide a API for other modules, thus 
offer a service. The client code will be invetable try to do stupid things 
because we're not living in a perfect world.

So please don't hide your input checks behind some #ifdefs 
like "XXX_EXTRA_DEBUG" or something similar. In general there should be no 
new #ifdefs introduced for (in my opinion stupid) things like this. Nobody 
will configure them, instead the default will be used. Every #ifdef increase 
the complexity, the number of code paths that needs to be tested, in the end 
the number of bugs. So again, instead of shifting the responsibility to the 
user or packager, try to find a solution that works generally.


2. Error logging

Its really essential to output meaningful error messages with the correct log 
level for problems that causes operations to fail. Its not feasible to run a 
production server with activated debug only to get information about the 
reason why this certain query failed. Error logs provides important 
information that is needed from API users, so please use LM_ERR and don't 
hide them under thousands of other meaningless debug messages. So again, if 
you return a failure to the client of the API, log also an error with a 
meaningful message that is understandable by the admin or user that don't 
wade through the code regularly. 


3. Code structure

Maintaining all this bunch of modules is much more easier if everybody tries 
to conform to a common coding style and structure. So please use the same 
conventions in your code that are used in other database modules, even if you 
don't like them. This applies to function definitions, parameter names, error 
logging, debug messages, even to the practise of memory management.

To be fair, this is only possible to a certain degree, but e.g. all SQL based 
driver library interfaces are somewhat similar, there was no reason in the 
first place why the SQL modules should differ that much. An additional 
benefit in having the same error and debug messages in different modules is 
that this really makes debugging and the bug fixing process easier. I know 
that you only care about "your" driver during development, but you should 
also care about the whole project. 


4. Documentation

Yes, nobody likes to write documentation. But the absolute minimum to 
understand any non-trivial foreign code is to have a comment block at the 
function definition describing in a few sentence the purpose of the code, the 
input parameter and return value. Its also important to describe why you do 
certain things in your code, e.g. because of some driver requirements, some 
bugs etc.. No comments in the code slows down the maintainance, more bugs 
will be introduced on changes and so on.


If we (as project) want to grow further in the future, we should seriously 
think about the conception of some coding standards and best practises in 
development, as most of this points also applies to other modules. Otherwise 
the code base will at some point in the future collapse under their own 
wight.


Best regards,

Henning


P.S: Code examples for the respective issues can be found at:

1. db_text and db_berkeley
2. db_text: dbt_base.c
3. db_mysql, db_unixodbc vs. db_postgres, now fixed
4. db_text and db_berkeley