[Asterisk-code-review] res pjsip pubsub.c: Implement "pjsip show subscriptions" com... (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/4747 )

Change subject: res_pjsip_pubsub.c: Implement "pjsip show subscriptions" commands.
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/4747/1/res/res_pjsip_pubsub.c
File res/res_pjsip_pubsub.c:

> This would not compile for me because there was no inclusion of asterisk/cl
Strange when jenkins was successful.


PS1, Line 3800: 	ast_str_append(&buf, 0, "Resource: %s\n", sub_tree->root->resource);
              : 	ast_str_append(&buf, 0, "Event: %s\n", sub_tree->root->handler->event_name);
              : 	ast_str_append(&buf, 0, "Expiry: %d\n", cli_subscription_expiry(sub_tree));
              : 
              : 	sip_subscription_to_ami(sub_tree, &buf);
              : 
              : 	/* Convert AMI \r\n to \n line terminators. */
              : 	src = strchr(ast_str_buffer(buf), '\r');
              : 	if (src) {
              : 		dest = src;
              : 		++src;
              : 		while (*src) {
              : 			if (*src == '\r') {
              : 				++src;
              : 				continue;
              : 			}
              : 			*dest++ = *src++;
              : 		}
              : 		*dest = '\0';
              : 		ast_str_update(buf);
              : 	}
              : 
              : 	ast_cli(cli->a->fd, "%s", ast_str_buffer(buf));
              : 	ast_free(buf);
> I hate to be that guy, but the output of this CLI command is completely inc
Requesting the information for a single subscription should be able to give more information than the show or list all subscription commands.  It seems kind of strange that the other show single item commands output the same information in the same format as the show all like this one item version.

It has to be this way for single subscriptions to get more information than the show/list multiple commands can output.  I just used the existing AMI generation API to get the subscription information.  Otherwise, I would have to implement a new or breaking API change into other modules just to get mailboxes or extension state information depending on the subscription type.


Line 4131: 		cli.like = ast_calloc(1, sizeof(*cli.like));
> Out of curiosity, why are cli.like and cli.buf heap-allocated? Is there a f
cli.like is a pointer to let the lower layer know if the output needs to be filtered by a regex.

As for the contents of cli.like being allocated, it doesn't have to be so I'll change that.  The example code I was using had allocated it.

cli.buf is a dynamic string because it is the output string generated for each subscription entry and I didn't want to have a problem with the buffer being too small.  It is only allocated once per command and reused per entry.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb8a3b61f447aedc58a8e6b36a810f7566018567
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes