[Asterisk-code-review] func odbc: single database connection should be optional (asterisk[13])

[Alexei Gradinari]

Alexei Gradinari has posted comments on this change.

Change subject: func_odbc: single database connection should be optional
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.asterisk.org/#/c/2800/4/funcs/func_odbc.c
File funcs/func_odbc.c:

Line 1755: 		ast_log(LOG_NOTICE, "Single database connection per DSN\n");
> If you are going to leave this in, please change this to a debug message. T
OK, will remove it


PS4, Line 1833: 	if (cfg && (s = ast_variable_retrieve(cfg, "general", "single_db_connection"))) {
              : 		single_db_connection = ast_true(s);
              : 	} else {
              : 		single_db_connection = DEFAULT_SINGLE_DB_CONNECTION;
              : 	}
              : 
              : 	if (single_db_connection && !dsns) {
              : 		dsns = ao2_container_alloc(DSN_BUCKETS, dsn_hash, dsn_cmp);
              : 		if (!dsns) {
              : 			ast_log(LOG_ERROR, "Could not initialize DSN container\n");
              : 			return 0;
              : 		}
              : 		ast_log(LOG_NOTICE, "Single database connection per DSN\n");
              : 	}
> This still has the potential for a race condition. Consider what happens if
I didn't find any documentation about locking functions.
What functions should I use to RD lock, RW lock, unlock?
ao2_rdlock, ao2_wrlock, ao2_unlock?
Can you provide example?


-- 
To view, visit https://gerrit.asterisk.org/2800
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I57d990616c957dabf7597dea5d5c3148f459dfb6
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-HasComments: Yes