[Asterisk-code-review] cel/mongodb: Added mongodb backend for cel (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: cel/mongodb: Added mongodb backend for cel
......................................................................


Patch Set 2: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/2361/2/cel/cel_mongodb.c
File cel/cel_mongodb.c:

Line 192: static int process_load_module(struct ast_config *cfg)
You should hold the mongodb_lock during this function. You are changing global variable values and reconnecting to the DB at the same time that other operations are potentially in flight.


Line 276:            ast_debug(3, "cel_mongodb: using default unix socket\n");
        :         } else {
        :             ast_debug(3, "cel_mongodb: got hostname of %s\n", hostname);
        :         }
        :         ast_debug(3, "cel_mongodb: got port of %s\n", dbport);
        :         ast_debug(3, "cel_mongodb: got user of %s\n", dbuser);
        :         ast_debug(3, "cel_mongodb: got dbname of %s\n", dbname);
        :         ast_debug(3, "cel_mongodb: got password of %s\n", password);
        :         ast_debug(3, "cel_mongodb: got collection name of %s\n", dbcollection);
        :         ast_debug(3, "cel_mongodb: got show_user_defined of %s\n",
The "cel_mongodb" prefixes are not necessary. The Asterisk logger can be configured to display the file name, function name, and log number of any given log message.


Line 301:         client = NULL;
This sets client NULL when it is already known to be NULL.


Line 326:     if (ast_cel_backend_register(MONGODB_BACKEND_NAME, mongodb_log)) {
        :         ast_log(LOG_WARNING, "Unable to subscribe to CEL events for mongodb\n");
        :         return AST_MODULE_LOAD_DECLINE;
        :     }
Only do this when "reload" is 0. Otherwise, you'll re-register the CEL backend every time the module is reloaded.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bf4f02fbc6a3a5edd7f10256ea0e14f49193c4f
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Catalin Stanciu <catacsdev at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes