[asterisk-dev] [Code Review] Fix misuses of ast_str

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2161/
-----------------------------------------------------------

Review request for Asterisk Developers.


Summary
-------

While writing the TCP tests for https://reviewboard.asterisk.org/r/2123 I accidentally misused ast_str when writing the test. I corrected my error and then realized it may be being misused other places in the code. Here's an example of the misuse I'm referring to:

int foo(struct ast_str *str)
{
    ast_str_set(&str, 0, "%s", some_string);
}

int bar(void)
{
    struct ast_str *str = ast_str_create(32);

    foo(str);

    do_something(str); /* DANGEROUS */
}

So let's go through this.
1) bar() creates an ast_str and calls foo(). 
2) foo() sets the ast_str to some value using ast_str_set(). 
3) ast_str_set() takes a pointer to the ast_str pointer because the ast_str can be reallocated. If this reallocation occurs, then when control returns to foo(), its version of str is pointing to invalid data.

The fix is to either 

1) Pass a struct (or other container) with the ast_str pointer to foo()
2) Pass a pointer to the ast_str pointer to foo()

In the cases changed in this diff, the second approach was taken since the ast_str pointers did not already belong to some struct.

While there are changes in app_dial.c, chan_iax2.c, and ccss.c, the only one that can be problematic in its current state is ccss.c. This is because in app_dial and chan_iax2, the ast_strs are created using ast_str_alloca() instead of ast_str_create(). This means they cannot be reallocated by a call to ast_str_set(). I elected to make the changes in app_dial and chan_iax2 because the functions as they exist have the potential for danger and should be made safer.


Diffs
-----

  /branches/1.8/apps/app_dial.c 374904 
  /branches/1.8/channels/chan_iax2.c 374904 
  /branches/1.8/main/ccss.c 374904 

Diff: https://reviewboard.asterisk.org/r/2161/diff


Testing
-------

Compiles, brah!


Thanks,

Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121012/97b2d01c/attachment.htm>