[Freeswitch-dev] Switch_core_get_variable

[Anthony Minessale]

I read this email then I went and solved it.
I see some replies came in.

Leaning on global vars is a bit of abuse but we can't afford to be
susceptible to a crash.
I have modified 26 files and changed all places to use one of the
following new functions:

char *switch_core_get_variable_dup(const char *var)
char *switch_core_get_variable_pdup(const char *var, switch_memory_pool_t *pool)

one strdups the value, the other pool dups it.

All that I ask is that you now review it to make sure I did not mess
up anything.



On Wed, Feb 2, 2011 at 3:48 PM, Kevin Snow <kevin.snow at ooma.com> wrote:
>
> I can enter it into jira.
>
> I’m not sure if I agree with your assessment. There is already a mutex
> lock/unlock around switch_core_get_variable’s internals today, no additional
> risk of deadlock. As for switch_channel_get_variable, which also has a mutex
> of it’s own, is already using switch_core_session_strdup before returning.
> I’m proposing the equivalent to switch_channel_get_variable_dup be added for
> switch_core_get_variable. To me it seems we must have this if you want to
> reliably read a variable that might change at runtime.
>
> Kevin
>
>
>
>
> On 2/2/11 12:01 PM, "Steven Ayre" <steveayre at gmail.com> wrote:
>
> I found a hole in the way we handle core variables in FS.
> Switch_core_get_variable does the lookup and returns the found pointer. In
> Switch_core_set_variable the first step it does is look up the variable and
> free it (if it exists). This would free it out from under another that has
> just done a get on it. This is how I stumbled on this.
>
> The correct place to report bugs is the bug
> tracker: http://jira.freeswitch.org/
>
> It's a known problem. Basically the answer is you shouldn't really be
> changing core variables after FS has loaded, at least not while handling
> traffic. If you're doing so as part of your application, you're doing it
> wrong. mod_db is probably better suited to what you want to do.
>
> Is the right fix is to add a switch_core_get_variable_dup that dups the
> string while in the mutex protection? I realize in the core case this will
> require the caller to then free the returned memory, but this is better than
> getting a bad pointer. This is analogous to the switch_channel_get_variable
> and it’s _dup implementation, although it dups it to session memory.
>
> Just my 2 cents (I'm not a core developer)... That could be done. But it's
> also slower because it means dynamically assigning memory and then copying
> the string. And it can't be cached. And the mutex will make all calls block,
> not just the single call as with the channel variable - which might possibly
> introduce a risk of deadlock.
>
> -Steve
>
>
>
> On 2 February 2011 19:39, Kevin Snow <kevin.snow at ooma.com> wrote:
>
> I found a hole in the way we handle core variables in FS.
> Switch_core_get_variable does the lookup and returns the found pointer. In
> Switch_core_set_variable the first step it does is look up the variable and
> free it (if it exists). This would free it out from under another that has
> just done a get on it. This is how I stumbled on this.
>
> Is the right fix is to add a switch_core_get_variable_dup that dups the
> string while in the mutex protection? I realize in the core case this will
> require the caller to then free the returned memory, but this is better than
> getting a bad pointer. This is analogous to the switch_channel_get_variable
> and it’s _dup implementation, although it dups it to session memory.
>
> Switch_channel_get_variable’s ability to peak through to the core variables
> is susceptible to this. If a core variable is changed after
> switch_channel_get_variable looks it up but before it dups to the session
> pool, it’ll have a bad pointer.
>
> Kevin Snow
> Ooma, Inc
>
> _______________________________________________
> FreeSWITCH-dev mailing list
> FreeSWITCH-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org
>
>
>
> ________________________________
> _______________________________________________
> FreeSWITCH-dev mailing list
> FreeSWITCH-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org
>
>
> _______________________________________________
> FreeSWITCH-dev mailing list
> FreeSWITCH-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org
>
>



-- 
Anthony Minessale II

FreeSWITCH http://www.freeswitch.org/
ClueCon http://www.cluecon.com/
Twitter: http://twitter.com/FreeSWITCH_wire

AIM: anthm
MSN:anthony_minessale at hotmail.com
GTALK/JABBER/PAYPAL:anthony.minessale at gmail.com
IRC: irc.freenode.net #freeswitch

FreeSWITCH Developer Conference
sip:888 at conference.freeswitch.org
googletalk:conf+888 at conference.freeswitch.org
pstn:+19193869900