[OpenSIPS-Devel] [ opensips-Bugs-3039943 ] db_mysql multiple issues in run_mysql_cmd wrapper

SourceForge.net noreply at sourceforge.net
Fri Dec 17 09:09:27 CET 2010


Bugs item #3039943, was opened at 2010-08-05 13:52
Message generated for change (Comment added) made by bogdan_iancu
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1086410&aid=3039943&group_id=232389

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 9
Private: No
Submitted By: Stanislaw Pitucha (viraptor)
Assigned to: Bogdan-Andrei Iancu (bogdan_iancu)
Summary: db_mysql multiple issues in run_mysql_cmd wrapper

Initial Comment:
There seem to be a couple of issues hidden in `run_mysql_cmd` macro:

- if there is no connection, first `if` causes a fall through to checking `mysql_errno()`, which has the return value of the last command, while it assumes checking the (_cmd)'s return code

- the macro is used for both prepared statement and other commands, even though mysql_stmt_* commands report the code via mysql_stmt_errno(), not mysql_errno()

- comparing mysql_errno to error codes and then getting mysql_error when error code==0 seems like a stupid thing to do, unless it's a workaround for some libmysqlclient bug

- "disconect" should be spelled "disconnect" :)

- reported errors are not true in functions using the wrapper - for example re_init_statement reports `if (code<0) { LM_ERR("failed while mysql_stmt_prepare()\n"); ...` while it can fail on db_mysql_connect which also returns the result via the 'code' variable

- if the database disconnectts during mysql_stmt_reppare, statements are going to be reset (reset_all_statements()), the final value of code will be that of db_mysql_connect (probably 0) and ctx->stmt will not be initialised, but mysql_stmt_close() will be run on it, causing a crash

- probably some others I haven't noticed...

Proposed solution - kill the macro and rewrite it in a sane way - I tried fixing the separate problems, but it seems like there are too many issues with it.

----------------------------------------------------------------------

>Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2010-12-17 10:09

Message:
Hi Stan,

Reviewed the fixes and look good - I uploaded your patches on trunk and
1.6

Many thanks & Best regards,
Bogdan

----------------------------------------------------------------------

Comment By: Stanislaw Pitucha (viraptor)
Date: 2010-08-11 12:18

Message:
All of them, to be applied in order. They're just split into the same
chunks, how I created / tested them. If it doesn't improve your review,
just apply all of them :)

----------------------------------------------------------------------

Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2010-08-11 11:23

Message:
Hi Stan,

Which patch is the final one? or all of them ?

Just to know where to start :)

Regards,
Bogdan

----------------------------------------------------------------------

Comment By: Stanislaw Pitucha (viraptor)
Date: 2010-08-06 14:48

Message:
Uploaded my patch queue. It removes all occurrences of run_mysql_cmd and
replaces them with some common code. (they can be macro'd if someone really
wants to do that)


More information about the Devel mailing list