<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13085">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/13085/1/apps/app_voicemail.c">File apps/app_voicemail.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/13085/1/apps/app_voicemail.c@5386">Patch Set #1, Line 5386:</a> <code style="font-family:monospace,monospace"> char fromdir[256], fromfile[256], origdate[80] = "", origcallerid[80] = "", origmailbox[80] = "";</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is "origmailbox" just the voicemail ID minus the context portion? I think it is. If so the length is probably fine. However, it would be better to use MAX_VM_MBOX_ID_LEN, or MAX_VM_MAILBOX_LEN (if includes '@context').</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/13085/1/apps/app_voicemail.c@5417">Patch Set #1, Line 5417:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> } else {<br> fprintf(p, "Dear %s:" ENDL ENDL "\tJust wanted to let you know you were just left a "<br> "%s long message (number %d)" ENDL "in mailbox %s from %s, on %s so you might" ENDL<br> "want to check it when you get a chance. Thanks!" ENDL ENDL "\t\t\t\t--Asterisk"<br> ENDL ENDL, vmu->fullname, dur, msgnum + 1, mailbox,<br> (cidname ? cidname : (cidnum ? cidnum : "an unknown caller")), date);<br> }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It appears the logic for outputting of this message has changed. Before the "plain message" was written if the given mailbox matched the vmu->mailbox, or the configuration couldn't load. Now it is only written if the given mailbox and configured mailbox match. Seems like you'd still want write it out if the configuration didn't load as well, or was this intentional?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13085">change 13085</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/13085"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Ife1c9ec8e7529b4d5da7f0ccf9b95cebcd5a6009 </div>
<div style="display:none"> Gerrit-Change-Number: 13085 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: cmaj <chris@penguinpbx.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 21 Oct 2019 18:49:41 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>