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:26 | Date Closed: | 2014-04-08 15:53:24 |
Priority: | Trivial | Regression? | No |
Status: | Closed/Complete | Components: | 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. |