<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1: Code-Review+1</p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably okay. However it'll miss reporting an error in at least one code path that it was previously doing. If you wanted you could log an error just before returning after the "tcptls_write_setup_error" label in the sip_tcptls_write function to catch that one.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also please cherry pick to the 16, 17, and master branches when ready.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">To be complete:</p><p style="white-space: pre-wrap; word-wrap: break-word;">There is two code paths that previously resulted in (incorrect) errors being printed (and after having finally read the code these code paths have caused me personally numerous hours of explaining that these errors are bogus, even though I only just "fixed" this on impulse after they caused me headaches trouble-shooting another (unrelated) issue due to flooding the logs.</p><p style="white-space: pre-wrap; word-wrap: break-word;">1.  if (!tcptls_session) .... obviously we can't write if we don't have a connection - so keep quiet about it now and if it's a reliable transmission hope the peer reconnects before it times out (nothing better we can do).<br>2.  The code path you mention is more complex, there are four possible failure modes:<br>2.1 fd == -1 - I'm assuming no connection - whould probably be treated like 1 above.<br>2.2 issues getting some thread - doesn't look like ao2_t_find will print an error ...<br>2.3 ao2_alloc error for packet; and <br>2.4 packet string alloc error - both presumably memory errors.  Not sure if they'll get logged.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I suspect the four paths for 2 should be split up, and more detailed logs be printed for each sub-branch, before going to the tcptls_write_setup_error label.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If you're happy with that as a solution, it'll end up that:</p><p style="white-space: pre-wrap; word-wrap: break-word;">1 and 2.1 never gets logged.  It'll be like sending a udp frame that gets lost on the wire somewhere.<br>2.2 will be logged as "Unable to locate tcptls helper thread"<br>2.3 and 2.4 will get logged as "memory allocation failure"</p><p style="white-space: pre-wrap; word-wrap: break-word;">Happy?</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13366">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13366">change 13366</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/+/13366"/><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: I46eb82924beeff9dfd86fa6c7eb87d2651b950f2 </div>
<div style="display:none"> Gerrit-Change-Number: 13366 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </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: Thu, 05 Dec 2019 18:43:24 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>