> On Sept. 4, 2014, 10:59 a.m., Matt Jordan wrote:
> > trunk/menuselect.c, lines 632-642
> > <https://reviewboard.asterisk.org/r/3975/diff/1/?file=67200#file67200line632>
> >
> >     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.

The debug line about positive output is valid because it's defined for all 
channel drivers as part of the MENUSELECT_CHANNELS category. It just defines 
which subset of items menuselect outputs (selected or not selected) and depends 
on how those who wrote the makefile chose to handle menuselect output (list of 
things to process or list of things to exclude from processing). I'll look for 
a better way to fix the bug.


- opticron


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


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