[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