[asterisk-dev] [Code Review]: Continuing from 1643 (Allow specifying which MixMonitor to Stop)

[jrose]

> On Jan. 20, 2012, 2:17 p.m., Mark Michelson wrote:
> > I'm in agreement with Richard with regards to trying to size the buffer for the pointer. It's a bad idea to try to assume how a system will stringify a pointer when using %p. There are a couple of ways around this:
> > 
> > 1. Size the buffer to something larger than you think is necessary, like 64 or something. Even still, I'd feel more comfortable using snprintf() for printing the value instead of sprintf(). And even still, this may not guarantee uniqueness if it by some bizarre reason turns out that a system prints its pointers some weird way, like in binary.
> > 2. Another way to guarantee uniqueness and know the size of the data you will store is to have an unsigned integer at file scope that increases by one each time you need a unique id. You can use ast_atomic_fetchadd_int to guarantee that no two threads end up with the same value. There are examples of this used in several places in Asterisk. I know off the top of my head that ccss.c uses this for identifying each of its core instances it allocates.

Kevin suggested specifically to use the pointer of the data store as the unique value, so I think I'll stick with that. I'll go ahead and change the multiplier for the buffer to 4 so that we have double the needed characters as a base, and also switch to snprintf, though I think I'll need to pass the length of the string to do that.


> On Jan. 20, 2012, 2:17 p.m., Mark Michelson wrote:
> > /trunk/apps/app_mixmonitor.c, lines 825-827
> > <https://reviewboard.asterisk.org/r/1682/diff/1/?file=23496#file23496line825>
> >
> >     This change seems unrelated to the addition of stopping a mixmonitor by ID. Can you explain why it's here?

Nope.  I can't.  Perhaps Telecos can.


- jrose


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


On Jan. 20, 2012, 1:07 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1682/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2012, 1:07 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and telecos.
> 
> 
> Summary
> -------
> 
> Fixes style problems from r6 on https://reviewboard.asterisk.org/r/1643/
> Changes creation of character buffer to length of a pointer in characters + 3 (2 for 0x, 1 for terminating space)
> 
> That might not be adequate.  Richard was saying something about pointers on other operating systems having other dividing symbols in them.  Well, at least ':'s.
> 
> In my experiences though, a pointer is usually just something like 0x0123FEDC when printed with %p.
> 
> 
> This addresses bug ASTERISK-19096.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19096
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 351538 
>   /trunk/apps/app_mixmonitor.c 351538 
> 
> Diff: https://reviewboard.asterisk.org/r/1682/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120120/3f5a6492/attachment-0001.htm>