[asterisk-dev] [Code Review] 3975: Menuselect: Fix incorrect enabling on failed deps

[Matt Jordan]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140904/e3c744e2/attachment.html>