-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3975/#review13233
-----------------------------------------------------------



trunk/menuselect.c
<https://reviewboard.asterisk.org/r/3975/#comment23723>

    I'm not sure I understand why this was broken.
    
    was_defaulted simply tracks whether or not there is a defaultenabled field. 
The first condition in this if statement is correct: if was_defaulted is true 
and there were no dependency failures, then we should set enabled based on the 
defaultenabled field.
    
    The problem seems to be in the second clause. If interactive is true, 
was_enabled should still be False - none of these modules should ever have been 
enabled. Your change makes us simply ignore was_enabled, and I'm not sure 
that's correct - or at least I don't understand what all will occur if we do 
ignore it.
    
    When I enabled menuselect debugging, I noticed that the code of lines 458 
was spitting out "Enabled (module) because the category does not have positive 
output". That means we are - for some reason - setting was_enabled on disabled 
modules when they were never enabled at all. I suspect that's more the cause of 
this issue then the logic here.


- Matt Jordan


On Sept. 4, 2014, 9:55 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3975/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2014, 9:55 a.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Repository: Menuselect
> 
> 
> Description
> -------
> 
> This corrects a situation where menuselect can incorrectly enable a module by 
> default that has defaultenabled set to "no" and has failed/non-selected 
> dependencies.
> 
> 
> Diffs
> -----
> 
>   trunk/menuselect.c 1186 
> 
> Diff: https://reviewboard.asterisk.org/r/3975/diff/
> 
> 
> Testing
> -------
> 
> Verified that the four modules that fit the requirements for this bug were 
> correctly not enabled when the patch was applied. The modules where I 
> observed this behavior are pbx_ael, chan_jingle, chan_gtalk, and chan_mgcp on 
> the 1.8.28 certified branch.
> 
> 
> Thanks,
> 
> opticron
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to