<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1365/">https://reviewboard.asterisk.org/r/1365/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks fine to me.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1365/diff/2/?file=18407#file18407line24963" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int get_cached_mwi(struct sip_peer *peer, int *new, int *old)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24963</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">oldmsgs</span> <span class="o">=</span> <span class="n">ast_event_get_ie_uint</span><span class="p">(</span><span class="n">event</span><span class="p">,</span> <span class="n">AST_EVENT_IE_OLDMSGS</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">24963</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* Fall back to manually checking the mailbox if not cache_only and get_cached_mwi failed */</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The comment did not need to change since what you added to the comment restates the code.</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On August 16th, 2011, 11:22 a.m., jrose wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Kevin Fleming, rmudgett, and mjordan.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Aug. 16, 2011, 11:22 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For the first issue, all I had to do was add the space character to the list of delimiter characters used by strsep. Actually, this change is somewhat questionable if spaces are normally allowed in mailbox names. I don't think they are, but I'm not certain. If they are, please let me know so that I can remove that change.
Alternatively, we could simply search for the , and trim any whitespace until the next character. Let me know if that's more ideal.
For the second issue, I removed use of event in the mwi notification building function because it would only ever check one mailbox and then skip the function to check cached events for the peer to tally the total number of old and new messages. I was a little wary of that, but after talking with Richard I was a little more confident in doing that.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested with sip.conf including spaces between mailboxes as well as commas and spaces. Now space can be used to separate mailboxes. Not sure whether that's a good thing or not, but it's easily fixable if it isn't.
Tested multiple mailboxes and single mailboxes to make sure subscribed mailboxes give mwi for the correct number of messages. Specific scenarios include at the actual time of subscription and when receiving new voicemail. All tests yielded appropriate voicemail counts in the mwi.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-15479">ASTERISK-15479</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-18067">ASTERISK-18067</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(331954)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1365/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>