> 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.
> 
> opticron wrote:
>     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.

I understand what you're saying now. Patch incoming.


- 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