Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 1:47 PM, George Joseph wrote: > On Tue, Sep 23, 2014 at 11:13 AM, Matthew Jordan wrote: >> >> >> >> On Tue, Sep 23, 2014 at 11:29 AM, Paul Belanger >> wrote: >>> >>> On Tue, Sep 23, 2014 at 11:45 AM, George Joseph >>> wrote: >>> > I've been working on some changes for config.c and in the process I've >>> > found >>> > 5 instances of someone attempting to do "cat->name == category_name" >>> > instead >>> > of "strcmp(cat->name, category_name)".Example: >>> > > > >>> >>> > My question is... Should I fix these so the case sensitive match works >>> > and >>> > runs first or just remove the first loop so the match is always >>> > case-insensitive? I'm hoping the latter not only because it makes the >>> > code >>> > simpler but because that's how it's worked for years and suddenly >>> > making the >>> > match case sensitive might cause unexpected problems. >>> > >>> > Thoughts? >>> > >>> For me, case sensitive. Because I config files that do have: >>> >>> [Foo] >>> >>> [foo] >>> >>> [fOO] >>> >>> don't ask, long story. >>> >> > > Which file? I'd like to test a before and after scenario to make sure > whatever I change still works. > My example was chan_sip, we mostly use MAC address for categories. I'd have to review my notes, but I actually think there is a bug (which you'll likely tell me), if you change the case of the category and reload chan_sip, asterisk does not detect the change. You have stop / start asterisk. >> >> And it currently works? :-D >> >> (Just kidding) >> >> If that is the case, then obviously any change we make here should be done >> to trunk only. >> >> I'll leave the impending case-war to others... :-) >> > > Ok, I think I'll fix the ones that are definitely broken so they do case > sensitive first then fall back to case insensitive. That should preserve > the current behavior. > > > -- > _ > -- 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 -- Paul Belanger | PolyBeacon, Inc. Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode) Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger -- _ -- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 11:13 AM, Matthew Jordan wrote: > > > On Tue, Sep 23, 2014 at 11:29 AM, Paul Belanger < > paul.belan...@polybeacon.com> wrote: > >> On Tue, Sep 23, 2014 at 11:45 AM, George Joseph >> wrote: >> > I've been working on some changes for config.c and in the process I've >> found >> > 5 instances of someone attempting to do "cat->name == category_name" >> instead >> > of "strcmp(cat->name, category_name)".Example: >> > >> > > > My question is... Should I fix these so the case sensitive match works >> and >> > runs first or just remove the first loop so the match is always >> > case-insensitive? I'm hoping the latter not only because it makes the >> code >> > simpler but because that's how it's worked for years and suddenly >> making the >> > match case sensitive might cause unexpected problems. >> > >> > Thoughts? >> > >> For me, case sensitive. Because I config files that do have: >> >> [Foo] >> >> [foo] >> >> [fOO] >> >> don't ask, long story. >> >> > Which file? I'd like to test a before and after scenario to make sure whatever I change still works. > And it currently works? :-D > > (Just kidding) > > If that is the case, then obviously any change we make here should be done > to trunk only. > > I'll leave the impending case-war to others... :-) > > Ok, I think I'll fix the ones that are definitely broken so they do case sensitive first then fall back to case insensitive. That should preserve the current behavior. -- _ -- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 10:05 AM, Richard Mudgett
wrote:
>
>
> On Tue, Sep 23, 2014 at 10:51 AM, George Joseph <
> george.jos...@fairview5.com> wrote:
>
>> On Tue, Sep 23, 2014 at 9:45 AM, George Joseph <
>> george.jos...@fairview5.com> wrote:
>>
>>> I've been working on some changes for config.c and in the process I've
>>> found 5 instances of someone attempting to do "cat->name == category_name"
>>> instead of "strcmp(cat->name, category_name)".Example:
>>>
>>> /* try exact match first, then case-insensitive match */
>>> for (cat = config->root; cat; cat = cat->next) {
>>> if (cat->name == category_name && (ignored || !cat->ignored))
>>> return cat;
>>> }
>>>
>>> for (cat = config->root; cat; cat = cat->next) {
>>> if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
>>> return cat;
>>> }
>>>
>>> The result is that the case sensitive match never succeeds and it's
>>> always the case insensitive match that's run.
>>>
>>> My question is... Should I fix these so the case sensitive match works
>>> and runs first or just remove the first loop so the match is always
>>> case-insensitive? I'm hoping the latter not only because it makes the
>>> code simpler but because that's how it's worked for years and suddenly
>>> making the match case sensitive might cause unexpected problems.
>>>
>>> Thoughts?
>>>
>>> Forgot to mention...There are other places in the code where the
>> comparison is always case-insensitive.
>>
>
> I was under the impression that the code was done this way
> initially to avoid doing a string comparison. Though, I'm not
> sure avoiding the string comparison really buys much in the
> way of performance anyway.
>
> Another reason is if you have several sections named [foo]
> and you need to resume with the same [foo] section as last
> time.
>
> Richard
>
> This works only in ast_category_browse where the pointer you're passing in
is almost always a pointer you received from the last call to
ast_category_browse.It's almost never going to work in ast_category_get
because you're probably passing in a constant or a pointer to some other
memory location housing the name.
--
_
-- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 11:29 AM, Paul Belanger <
paul.belan...@polybeacon.com> wrote:
> On Tue, Sep 23, 2014 at 11:45 AM, George Joseph
> wrote:
> > I've been working on some changes for config.c and in the process I've
> found
> > 5 instances of someone attempting to do "cat->name == category_name"
> instead
> > of "strcmp(cat->name, category_name)".Example:
> >
> > /* try exact match first, then case-insensitive match */
> > for (cat = config->root; cat; cat = cat->next) {
> > if (cat->name == category_name && (ignored || !cat->ignored))
> > return cat;
> > }
> >
> > for (cat = config->root; cat; cat = cat->next) {
> > if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
> > return cat;
> > }
> >
> > The result is that the case sensitive match never succeeds and it's
> always
> > the case insensitive match that's run.
> >
> > My question is... Should I fix these so the case sensitive match works
> and
> > runs first or just remove the first loop so the match is always
> > case-insensitive? I'm hoping the latter not only because it makes the
> code
> > simpler but because that's how it's worked for years and suddenly making
> the
> > match case sensitive might cause unexpected problems.
> >
> > Thoughts?
> >
> For me, case sensitive. Because I config files that do have:
>
> [Foo]
>
> [foo]
>
> [fOO]
>
> don't ask, long story.
>
>
And it currently works? :-D
(Just kidding)
If that is the case, then obviously any change we make here should be done
to trunk only.
I'll leave the impending case-war to others... :-)
--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
--
_
-- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 11:45 AM, George Joseph
wrote:
> I've been working on some changes for config.c and in the process I've found
> 5 instances of someone attempting to do "cat->name == category_name" instead
> of "strcmp(cat->name, category_name)".Example:
>
> /* try exact match first, then case-insensitive match */
> for (cat = config->root; cat; cat = cat->next) {
> if (cat->name == category_name && (ignored || !cat->ignored))
> return cat;
> }
>
> for (cat = config->root; cat; cat = cat->next) {
> if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
> return cat;
> }
>
> The result is that the case sensitive match never succeeds and it's always
> the case insensitive match that's run.
>
> My question is... Should I fix these so the case sensitive match works and
> runs first or just remove the first loop so the match is always
> case-insensitive? I'm hoping the latter not only because it makes the code
> simpler but because that's how it's worked for years and suddenly making the
> match case sensitive might cause unexpected problems.
>
> Thoughts?
>
For me, case sensitive. Because I config files that do have:
[Foo]
[foo]
[fOO]
don't ask, long story.
--
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger
--
_
-- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 10:51 AM, George Joseph wrote:
> On Tue, Sep 23, 2014 at 9:45 AM, George Joseph <
> george.jos...@fairview5.com> wrote:
>
>> I've been working on some changes for config.c and in the process I've
>> found 5 instances of someone attempting to do "cat->name == category_name"
>> instead of "strcmp(cat->name, category_name)".Example:
>>
>> /* try exact match first, then case-insensitive match */
>> for (cat = config->root; cat; cat = cat->next) {
>> if (cat->name == category_name && (ignored || !cat->ignored))
>> return cat;
>> }
>>
>> for (cat = config->root; cat; cat = cat->next) {
>> if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
>> return cat;
>> }
>>
>> The result is that the case sensitive match never succeeds and it's
>> always the case insensitive match that's run.
>>
>> My question is... Should I fix these so the case sensitive match works
>> and runs first or just remove the first loop so the match is always
>> case-insensitive? I'm hoping the latter not only because it makes the
>> code simpler but because that's how it's worked for years and suddenly
>> making the match case sensitive might cause unexpected problems.
>>
>> Thoughts?
>>
>> Forgot to mention...There are other places in the code where the
> comparison is always case-insensitive.
>
I was under the impression that the code was done this way
initially to avoid doing a string comparison. Though, I'm not
sure avoiding the string comparison really buys much in the
way of performance anyway.
Another reason is if you have several sections named [foo]
and you need to resume with the same [foo] section as last
time.
Richard
--
_
-- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 10:45 AM, George Joseph wrote:
> I've been working on some changes for config.c and in the process I've
> found 5 instances of someone attempting to do "cat->name == category_name"
> instead of "strcmp(cat->name, category_name)".Example:
>
> /* try exact match first, then case-insensitive match */
> for (cat = config->root; cat; cat = cat->next) {
> if (cat->name == category_name && (ignored || !cat->ignored))
> return cat;
> }
>
> for (cat = config->root; cat; cat = cat->next) {
> if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
> return cat;
> }
>
> The result is that the case sensitive match never succeeds and it's always
> the case insensitive match that's run.
>
> My question is... Should I fix these so the case sensitive match works
> and runs first or just remove the first loop so the match is always
> case-insensitive? I'm hoping the latter not only because it makes the
> code simpler but because that's how it's worked for years and suddenly
> making the match case sensitive might cause unexpected problems.
>
>
>
My vote is for case insensitive.
In fact, practically speaking, I'm not sure how this isn't always case
insensitive.
Given contexts [foo] and [Foo], searching for "foo":
* If foo matches context foo directly, return foo.
* If foo matches context Foo, return Foo.
Either way, you'll end up matching on Foo eventually.
If there's a risk here that I'm missing, then I'd vote trunk only for
changes.
--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
--
_
-- 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
Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names
On Tue, Sep 23, 2014 at 9:45 AM, George Joseph
wrote:
> I've been working on some changes for config.c and in the process I've
> found 5 instances of someone attempting to do "cat->name == category_name"
> instead of "strcmp(cat->name, category_name)".Example:
>
> /* try exact match first, then case-insensitive match */
> for (cat = config->root; cat; cat = cat->next) {
> if (cat->name == category_name && (ignored || !cat->ignored))
> return cat;
> }
>
> for (cat = config->root; cat; cat = cat->next) {
> if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
> return cat;
> }
>
> The result is that the case sensitive match never succeeds and it's always
> the case insensitive match that's run.
>
> My question is... Should I fix these so the case sensitive match works
> and runs first or just remove the first loop so the match is always
> case-insensitive? I'm hoping the latter not only because it makes the
> code simpler but because that's how it's worked for years and suddenly
> making the match case sensitive might cause unexpected problems.
>
> Thoughts?
>
> Forgot to mention...There are other places in the code where the
comparison is always case-insensitive.
--
_
-- 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
[asterisk-dev] Opinions Needed: Case sensitivity in config file section names
I've been working on some changes for config.c and in the process I've
found 5 instances of someone attempting to do "cat->name == category_name"
instead of "strcmp(cat->name, category_name)".Example:
/* try exact match first, then case-insensitive match */
for (cat = config->root; cat; cat = cat->next) {
if (cat->name == category_name && (ignored || !cat->ignored))
return cat;
}
for (cat = config->root; cat; cat = cat->next) {
if (!strcasecmp(cat->name, category_name) && (ignored || !cat->ignored))
return cat;
}
The result is that the case sensitive match never succeeds and it's always
the case insensitive match that's run.
My question is... Should I fix these so the case sensitive match works and
runs first or just remove the first loop so the match is always
case-insensitive? I'm hoping the latter not only because it makes the
code simpler but because that's how it's worked for years and suddenly
making the match case sensitive might cause unexpected problems.
Thoughts?
--
_
-- 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
