[Asterisk-code-review] time: add support for time64 libcs (asterisk[16])
Joshua Colp
asteriskteam at digium.com
Fri Mar 4 10:06:50 CST 2022
Attention is currently required from: Philip Prindeville.
Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18123 )
Change subject: time: add support for time64 libcs
......................................................................
Patch Set 2: Code-Review-1
(5 comments)
File include/asterisk/time.h:
https://gerrit.asterisk.org/c/asterisk/+/18123/comment/36aa5f2e_4e750e29
PS2, Line 330: time_t ast_string_to_time_t(const char *str);
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.
File main/time.c:
https://gerrit.asterisk.org/c/asterisk/+/18123/comment/bde2a64f_3e176de7
PS2, Line 152: int ast_time_t_to_string(time_t time, char *buf, size_t length)
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?
https://gerrit.asterisk.org/c/asterisk/+/18123/comment/44588a5d_762a94a8
PS2, Line 156: (void)localtime_r(&time, &tm);
Why the void?
https://gerrit.asterisk.org/c/asterisk/+/18123/comment/6da01cf6_e1e8b282
PS2, Line 168: if (strptime(str, " %s", &tm) == NULL)
We enclose ifs in { }
File res/res_calendar_caldav.c:
https://gerrit.asterisk.org/c/asterisk/+/18123/comment/c603f67b_9e21f06b
PS2, Line 407: char tmp[24];
Why 24 across the variables in this review?
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18123
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Id7b25bdca8f92e34229f6454f6c3e500f2cd6f56
Gerrit-Change-Number: 18123
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Prindeville <philipp at redfish-solutions.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Philip Prindeville <philipp at redfish-solutions.com>
Gerrit-Comment-Date: Fri, 04 Mar 2022 16:06:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220304/fe6d9e1d/attachment-0001.html>
More information about the asterisk-code-review
mailing list