[Asterisk-code-review] Restrict CLI/AMI commands on shutdown. (asterisk[13])

Mark Michelson asteriskteam at digium.com
Thu Feb 25 16:03:17 CST 2016


Mark Michelson has posted comments on this change.

Change subject: Restrict CLI/AMI commands on shutdown.
......................................................................


Patch Set 1:

(2 comments)

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

Line 2686: 	AST_CLI_DEFINE(handle_restart_now, "Restart Asterisk immediately"),
> I assumed that you allow stop now so you can immediately stop if you previo
Your assumption is correct. I see what you're saying now. If you've started a graceful restart, then allowing a "now" restart to override makes good sense. I'll alter it to work that way.


https://gerrit.asterisk.org/#/c/2283/1/main/cli.c
File main/cli.c:

Line 2702
> I think this increment needs to be done with &helpers locked to prevent a r
I see where you're coming from, but I don't think this will actually help. __ast_cli_unregister checks the value of e->inuse without helpers locked. In addition, the decrement of e->inuse further down in this function is performed without helpers locked. So even if I increment e->inuse with helpers locked, I can still have the command get unregistered out from under me.

The use of this counter here actually seems like a crude implementation of reference counts that probably dates back to pre-astobj2. I think it should probably be updated, but I also think that this is outside the scope of this particular changeset.

I can either leave this like I have it, or I can revert this increment to be where it wasand alter the "done" label to decrement e->inuse. In my mind it's six on one hand, half a dozen on the other, but if one seems better to you, then please let me know and I'll go with that choice.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f5b8e665bd4d0108014a8b6589729ecd3677eef
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
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



More information about the asterisk-code-review mailing list