<p> Attention is currently required from: Joshua Colp. </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/7288c776_55400974">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;">This is consistent with the mtkime() interface which states:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">The mktime() function returns the specified calendar time; if the calendar time cannot be represented, it returns -1;</p></blockquote></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/18b26dd1_93c8b25c">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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">In other APIs we use a thread local storage buffer so the caller doesn't need a buffer, is it possib […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I was following the model of ast_format_duration_hh_mm_ss().</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/a32feb77_4fb5235b">Patch Set #2, Line 156:</a> <code style="font-family:monospace,monospace">      (void)localtime_r(&time, &tm);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why the void?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">To make it clear that the return value is being discarded.  localtime_r() returns the value deposited into its pass-by-reference parameter.  Since it's passed into &tm, we don't need the return pointer.  Also didn't want -Wunused-result generating a frivolous warning.</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/a937a078_20edb5aa">Patch Set #2, Line 168:</a> <code style="font-family:monospace,monospace">      if (strptime(str, " %s", &tm) == NULL)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">We enclose ifs in { }</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/d5ec1e65_db49b1a1">Patch Set #2, Line 407:</a> <code style="font-family:monospace,monospace">                  char tmp[24];</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why 24 across the variables in this review?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">philipp@ubuntu20:~/asterisk$ cat /tmp/foo.c<br>#include <stdio.h><br>#include <string.h><br>#include <inttypes.h><br>#include <stdlib.h></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">int main(int argc, char *argv)<br>{<br>     char buf[100];</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">        snprintf(buf, sizeof(buf), "%lu", UINT64_MAX);<br>      printf("%zd\n", strlen(buf) + 1);<br>   exit(0);<br>}</pre><p style="white-space: pre-wrap; word-wrap: break-word;">philipp@ubuntu20:~/asterisk$ gcc -o foo /tmp/foo.c<br>philipp@ubuntu20:~/asterisk$ ./foo<br>21<br>philipp@ubuntu20:~/asterisk$</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: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 04 Mar 2022 16:48:03 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>