[Home]

Summary:ASTERISK-23546: CB_ADD_LEN does not do what you'd think
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2014-03-27 09:04:26Date Closed:2014-04-08 15:53:24
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Core/Configuration Core/General
Versions:SVN 1.8.26.1 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:What's wrong with this code? ;)

{code}
static void  CB_ADD_LEN(struct ast_str **cb, const char *str, int len)
{
       char *s = ast_alloca(len + 1);
       ast_copy_string(s, str, len);
       ast_str_append(cb, 0, "%s", str);
}
{code}
Comments:By: Rusty Newton (rnewton) 2014-04-02 18:39:48.655-0500

Can you add the component or show where this problem is?  I could go grep the source, but you reported the issue. :)

Can you also describe what the issue is with this code, or else describe what issue this might cause for a user of Asterisk if any?

The description isn't helpful for a non-developer. Thanks!

By: Richard Mudgett (rmudgett) 2014-04-02 19:11:16.513-0500

The line should read: :)
{noformat}
ast_str_append(cp, 0, "%s", s);
{noformat}


By: Walter Doekes (wdoekes) 2014-04-03 01:58:59.018-0500

Well.. there is only one place that it is used.
{noformat}
asterisk-1.8.x-WRITE$ wgrep . CB_ADD_LEN
./main/config.c:static void  CB_ADD_LEN(struct ast_str **cb, const char *str, int len)
./main/config.c: CB_ADD_LEN(&comment_buffer, oldptr+1, new_buf-oldptr-1);
{noformat}

And then there is the time machine in {{utils/extconf.c}}, which shows what it should do when it still worked:
{noformat}
./utils/extconf.c:static void  CB_ADD_LEN(char *str, int len)
./utils/extconf.c: CB_ADD_LEN(oldptr+1,new_buf-oldptr-1);
{noformat}

Apparently the main/config version can be safely replaced with CB_ADD, since that has been real behaviour for a while.
{noformat}
static void  CB_ADD(struct ast_str **cb, const char *str)
{
       ast_str_append(cb, 0, "%s", str);
}
{noformat}

Option A: remove CB_ADD_LEN and pretend it never existed.
Option B: find out why no one has noticed that it was broken and fix and check that it doesn't cause regressions for anyone.

By: Rusty Newton (rnewton) 2014-04-03 19:05:37.818-0500

Thanks guys!

By: Richard Mudgett (rmudgett) 2014-04-08 15:26:43.820-0500

No one has noticed this bug because I don't think there are many people using (or even know about) the ;-- --; style comment blocks.  This code is only executed when the comments are being saved and the block comment starts and ends within the same line like this:
{noformat}
exten => 1000,;-- Comment --;1,NoOp()
{noformat}

The example above requires the CB_ADD_LEN() code to save the comment portion correctly and not the CB_ADD() behaviour bug.

By: Richard Mudgett (rmudgett) 2014-04-08 15:37:14.082-0500

There is another bug if you have the following line:
{noformat}
exten => 1000,;-- Comment 1 ;-- Nested comment --; back in outer comment. --;1,NoOp()
{noformat}

The comment portion is not going to be saved correctly.  I don't think it is really worth fixing though.