[Asterisk-code-review] time: add support for time64 libcs (asterisk[16])

Philip Prindeville asteriskteam at digium.com
Fri Mar 4 10:48:03 CST 2022


Attention is currently required from: Joshua Colp.
Philip Prindeville has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18123 )

Change subject: time: add support for time64 libcs
......................................................................


Patch Set 2:

(5 comments)

File include/asterisk/time.h:

https://gerrit.asterisk.org/c/asterisk/+/18123/comment/7288c776_55400974 
PS2, Line 330: time_t ast_string_to_time_t(const char *str);
This is consistent with the mtkime() interface which states:

> The mktime() function returns the specified calendar time; if the calendar time cannot be represented, it returns -1;


File main/time.c:

https://gerrit.asterisk.org/c/asterisk/+/18123/comment/18b26dd1_93c8b25c 
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 possib […]
I was following the model of ast_format_duration_hh_mm_ss().


https://gerrit.asterisk.org/c/asterisk/+/18123/comment/a32feb77_4fb5235b 
PS2, Line 156: 	(void)localtime_r(&time, &tm);
> Why the void?
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.


https://gerrit.asterisk.org/c/asterisk/+/18123/comment/a937a078_20edb5aa 
PS2, Line 168: 	if (strptime(str, " %s", &tm) == NULL)
> We enclose ifs in { }
Done


File res/res_calendar_caldav.c:

https://gerrit.asterisk.org/c/asterisk/+/18123/comment/d5ec1e65_db49b1a1 
PS2, Line 407: 			char tmp[24];
> Why 24 across the variables in this review?
philipp at ubuntu20:~/asterisk$ cat /tmp/foo.c
#include <stdio.h>
#include <string.h>
#include <inttypes.h>
#include <stdlib.h>

int main(int argc, char *argv)
{
	char buf[100];

	snprintf(buf, sizeof(buf), "%lu", UINT64_MAX);
	printf("%zd\n", strlen(buf) + 1);
	exit(0);
}

philipp at ubuntu20:~/asterisk$ gcc -o foo /tmp/foo.c
philipp at ubuntu20:~/asterisk$ ./foo
21
philipp at ubuntu20:~/asterisk$



-- 
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: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Fri, 04 Mar 2022 16:48:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220304/9d793e6b/attachment-0001.html>


More information about the asterisk-code-review mailing list