[Asterisk-code-review] ARI: Added new functionality to get all module information. (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: ARI: Added new functionality to get all module information.
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/732/1/include/asterisk/module.h
File include/asterisk/module.h:

Line 164:  * /brief Ask for a list of modules, descriptions, use counts and status.
\brief instead of /brief


https://gerrit.asterisk.org/#/c/732/1/main/loader.c
File main/loader.c:

Line 1433: 	struct ast_module *cur;
         : 	int unlock = -1;
         : 	int total_mod_loaded = 0;
         : 	AST_LIST_HEAD_NOLOCK(, ast_module) alpha_module_list = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
         : 
         : 	if (AST_DLLIST_LOCK(&module_list)) {
         : 		unlock = 0;
         : 	}
         : 
         : 	AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
         : 		AST_LIST_INSERT_SORTALPHA(&alpha_module_list, cur, list_entry, resource);
         : 	}
         : 
         : 	while ((cur = AST_LIST_REMOVE_HEAD(&alpha_module_list, list_entry))) {
         : 		total_mod_loaded += modentry(cur->resource, cur->info->description, cur->usecount,
         : 		        cur->flags.running? "Running" : "Not Running", like, cur->info->support_level, data);
         : 	}
         : 
         : 	if (unlock) {
         : 		AST_DLLIST_UNLOCK(&module_list);
         : 	}
         : 
         : 	return total_mod_loaded;
I have a bit of a problem with this function's implementation. It basically is a copy of ast_update_module_list() except that it has the extra "data" parameter in the function and in the modentry callback. Rather than having a copy of the function, I think it would be better if the two separate functions existed, but were implemented in terms of each other. This may be a touch tricky because of the difference in modentry function pointers, but eliminating the duplicate code makes for much easier maintenance.


https://gerrit.asterisk.org/#/c/732/1/res/ari/resource_asterisk.c
File res/ari/resource_asterisk.c:

Line 157: static int process_module_list(const char *module, const char *description, int usecnt,
This function is not being called anywhere.


Line 163: 	module_info = ast_json_pack("{s: s, s: s, s: i, s: s, s: s}",
        : 															"name", module,
        : 															"description", description,
        : 															"use_count", usecnt,
        : 															"status", status,
        : 													
It looks like something got messed up here. This is missing the arguments for the ast_json_pack(), and there are a bunch of lines consisting of a bunch of tabs, only.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I63cbbf0ec0c3544cc45ed2a588dceabe91c5e0b0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes