Free database column name as part of database result cleanup

[quentusrex]

The column name is currently not free'd. Some db bankends copy this
data so that memory is leaked. Some store internal database pointers
and those shouldn't be free'd. One returns a pointer to a stack
variable which shouldn't be done.

The patch cleans up all db backends to copy the column name and frees
that column name as part of the database result cleanup function.

Author: Chris Double doublec@silentcircle.com

[miconda]

I don't really like this approach. The column names are not used inside the modules that conenct to the database, is only sqlops that exposes them and can be accessed in the configuration file. So I would not like to clone the column names if they are not necessary, some db operations are done quite frequest (e.g., auth db).

You haven't named the modules with issues, just some generic description that some do and some don't. I am not saying that there are not issues, but they need to be identified properly and see what is the best fix for it. I manly use mysql module for databases (and some bits of mongo).

I remember there was a similar patch back in August-September last year more or less trying to address some issues with the Postgres module, but afaik there was a different patch.

Anyhow, there can be indeed cases when the column name must be cloned. For this case I would go with the approach done for db result values. A flag is kept for each to indicate whether it has to be freed or not. That means adding a flags or similar field in db column structure that will be set by each db module if they do cloning of the column name. I can add this to the core and you can come with the patch for the module that really needs this cloning.

Of course, comments about my suggestion and other proposals are welcome.

[quentusrex]

@miconda Makes sense. No need to malloc/free if it isn't absolutely required for a commonly used code path. I'll look over the patch and see if I can see a better approach(using flags to signify malloced pointer). One of the larger concerns here was that clang's address sanitizer was pointing out that at least one of the cases there was a pointer to stack memory being returned, which could lead to bad things as that stack frame was used by another thread.

[miconda]

@quentusrex - is it ok to pursue the way with the flag for column name allocation (following the patch referenced above)? If so, I will close this pull request.

[quentusrex]

I'll update the patch and open a new PR when it's ready.