Hi,
Under all circumstances, the patch is better than the one I was forced
to make myself in a hurry when I encountered the problem. I ended up not
using the real_escape version because I didn't have time to dig into how
the DB pool was working, and I didn't use the Octstr struct.
This is also why I didn't offered you my own patch. You would probably
spend more time fixing my patch, than writing your own :D
But it is good to see that fixes are posted this fast. With certain
other projects, it may take months before you even get a response!
Best regards
Peter Christensen
(ungod in the bug report system)
Stipe Tolj wrote:
Rene Kluwen wrote:
IMHO consuming DBPoolConns is not a bad thing. After all, that is why the
pool even exist. So consuming and producing sql connections is not so
expensive.
ok, that's true. The main issue is that in some circumstances you may
need 2 connections for "one-and-the-same" job, where 1 would be enought.
Think of thread overlays. Now, on the other hand I agree and actually we
shouldn't make life inside dlr_mysql too hard in order to pass mysql
connection structs to lower function call layers.
In your patch: Where exactly do you escpape the parameters to the sql
string? I couldnt find that so quick.
I use mysql_real_escape_string() within gw/dlr_mysql.c:mysql_escape().
In most of the Chimit code, we use something like this:
Octstr *sql = octstr_sql_format("SELECT * from table where pk = '%s'",
PrimaryKey);
The function octstr_sql_format is crafted so that before replacing the
value
of PrimaryKey on the place that says %s in the format string,
automatically
it is sql-escaped.
This way, you never have to worry about escaping stuff - everything goes
automagically. This a clean and generic solution that generates little
coding effort.
yep, +1 on this approach... but I'd see the octstr_sql_format() renamed
to mysql_octstr_format() in order to semantically scope this to the
mysql layer and not fitting it to the gwlib/ itself.
Another thing. It seems to me that you use "only" mysql_escape_string()
from the mysql client lib here, right? You don't use
mysql_real_escapee_string() which implies that you aren't aware of the
charset encoding and still the escaping "could fail". At least most of
the significant projects using the mysql client lib changed to call the
mysql_real_... function instead of the other one. Even the mysql docs
advice this to do.
The whole dilema about the previously needed DBPoolConn wouldn't be
necessary if we "simply" pick the mysql_escape_string() instead of the
sophisticated one ;)
Stipe
mailto:stolj_{at}_wapme-group.de
-------------------------------------------------------------------
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
-------------------------------------------------------------------
