<p> Attention is currently required from: Philip Prindeville. </p>
<p>Patch set 2:<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/+/18123">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/time.h:</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/asterisk/+/18123/comment/36aa5f2e_4e750e29">Patch Set #2, Line 330:</a> <code style="font-family:monospace,monospace">time_t ast_string_to_time_t(const char *str);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not a fan of API calls that return special values like -1, I think passing in time_t * and then returning 0 or -1 for success and failure would be nicer.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/time.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/asterisk/+/18123/comment/bde2a64f_3e176de7">Patch Set #2, Line 152:</a> <code style="font-family:monospace,monospace">int ast_time_t_to_string(time_t time, char *buf, size_t length)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In other APIs we use a thread local storage buffer so the caller doesn't need a buffer, is it possible to do that here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18123/comment/44588a5d_762a94a8">Patch Set #2, Line 156:</a> <code style="font-family:monospace,monospace">   (void)localtime_r(&time, &tm);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why the void?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18123/comment/6da01cf6_e1e8b282">Patch Set #2, Line 168:</a> <code style="font-family:monospace,monospace">  if (strptime(str, " %s", &tm) == NULL)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">We enclose ifs in { }</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_calendar_caldav.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/asterisk/+/18123/comment/c603f67b_9e21f06b">Patch Set #2, Line 407:</a> <code style="font-family:monospace,monospace">                      char tmp[24];</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why 24 across the variables in this review?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18123">change 18123</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/+/18123"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: Id7b25bdca8f92e34229f6454f6c3e500f2cd6f56 </div>
<div style="display:none"> Gerrit-Change-Number: 18123 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Philip Prindeville <philipp@redfish-solutions.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Philip Prindeville <philipp@redfish-solutions.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 04 Mar 2022 16:06:50 +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>