From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id E7FDD6EC40; Sun, 8 Aug 2021 17:34:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E7FDD6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628433279; bh=aVgRp1ytoNM9WLfavT0Illmc1M9/8RncPMNry1i9Ld4=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=O7UzCechDSkS/tUSIUAqOh21MmjutfD+DhPCTlXa1YqF6Mw1dx+48gL4aNz0JY38h 7AaNkoCVtYlf9xliZjqthU9XEh3ovTeVKapmcG3jfsvoPuqFzpLOAqS7TtI6IlGEgm 9B5hhfRSG5sPaCqFn9RTnrwhWq4xK9Qdb8pqDDPM= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id AB24B6EC40 for ; Sun, 8 Aug 2021 17:34:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AB24B6EC40 Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1mCjsp-00045s-PO; Sun, 08 Aug 2021 17:34:36 +0300 To: Timur Safin , olegrok@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: Message-ID: <8279118f-bcbb-ccbd-375f-3b8f88ffdfb2@tarantool.org> Date: Sun, 8 Aug 2021 17:34:35 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9BCE6B93DE0C6C3914462CDB1732D383C182A05F538085040919622AE7E9CAD11216021D55766EBE3BC2516377FD66A00F76F107922118A6A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE742D9BD90C58D50E0EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063757B4903363AB6D598638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D84EAA49D9DFAAD77BA8AF6DC8A3FF1439117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE0AC5B80A05675ACD03F1AB874ED890284AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C36E36DCD5FF651F90BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7E1BCFB2C0BE3F189731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505BE6F59871C207F55B464E56AE7839C53 X-C1DE0DAB: 0D63561A33F958A50FB5029425ABB304E239B08D235ADB0CF764F4F4E7550BD3D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75AF0B556A5A327A45410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3457FA942CB4462B4CD9F264E3ACACE047924398E1F3AD314358FA1AD82B6E3863971369148326CF901D7E09C32AA3244CD2163521369F5EDDA23F3E07065CA6DB64EE5813BBCA3A9DFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojMTMPlNJj3SiJdxp2/2Ub4Q== X-Mailru-Sender: 6C3E74F07C41AE94D32402E5012278FAF845936906850D5BDBDB9F18EFF1BE3E017DAA252354DF0E07784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for the patch! See 7 comments below. > diff --git a/src/exports.h b/src/exports.h > index 5bb3e6a2b..fb4a8084f 100644 > --- a/src/exports.h > +++ b/src/exports.h > @@ -217,7 +217,32 @@ EXPORT(curl_url_set) > EXPORT(curl_version) > EXPORT(curl_version_info) > #endif /* EXPORT_LIBCURL_SYMBOLS */ > +EXPORT(datetime_asctime) > +EXPORT(datetime_ctime) > +EXPORT(datetime_now) > +EXPORT(datetime_strftime) > EXPORT(decimal_unpack) > +EXPORT(dt_dow) > +EXPORT(dt_from_rdn) > +EXPORT(dt_from_struct_tm) > +EXPORT(dt_from_yd) > +EXPORT(dt_from_ymd) > +EXPORT(dt_from_yqd) > +EXPORT(dt_from_ywd) > +EXPORT(dt_parse_iso_date) > +EXPORT(dt_parse_iso_time_basic) > +EXPORT(dt_parse_iso_time_extended) > +EXPORT(dt_parse_iso_time) > +EXPORT(dt_parse_iso_zone_basic) > +EXPORT(dt_parse_iso_zone_extended) > +EXPORT(dt_parse_iso_zone_lenient) > +EXPORT(dt_parse_iso_zone) > +EXPORT(dt_rdn) > +EXPORT(dt_to_struct_tm) > +EXPORT(dt_to_yd) > +EXPORT(dt_to_ymd) > +EXPORT(dt_to_yqd) > +EXPORT(dt_to_ywd) 1. In scope of https://github.com/tarantool/tarantool/pull/6265 the exported symbols are heavily reworked. In particular, you should not export library symbols as is, you need to wrap them into tt_cdt_* functions and export them instead. Otherwise the users' modules, which link with their own version of c-dt, will have conflicts. > EXPORT(error_ref) > EXPORT(error_set_prev) > EXPORT(error_unpack_unsafe) > diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c > new file mode 100644 > index 000000000..e77b188b7 > --- /dev/null > +++ b/src/lib/core/datetime.c > @@ -0,0 +1,96 @@ > +/* > + * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include > +#include > + > +#include "trivia/util.h" > +#include "datetime.h" > + > +static int > +local_dt(int64_t secs) > +{ > + return dt_from_rdn((int)(secs / SECS_PER_DAY) + DT_EPOCH_1970_OFFSET); > +} > + > +static struct tm * > +datetime_to_tm(const struct datetime *date) > +{ > + static struct tm tm; > + > + memset(&tm, 0, sizeof(tm)); 2. Why do you need this memset if you overwrite all the fields anyway? > + int64_t secs = date->secs; > + dt_to_struct_tm(local_dt(secs), &tm); > + > + int seconds_of_day = date->secs % SECS_PER_DAY; > + tm.tm_hour = (seconds_of_day / 3600) % 24; > + tm.tm_min = (seconds_of_day / 60) % 60; > + tm.tm_sec = seconds_of_day % 60; > + > + return &tm; > +} > + > +void > +datetime_now(struct datetime * now) 3. Please, do not use whitespace between * and the variable name. > +{ > + struct timeval tv; > + gettimeofday(&tv, NULL); > + now->secs = tv.tv_sec; > + now->nsec = tv.tv_usec * 1000; > + > + time_t now_seconds; > + time(&now_seconds); > + struct tm tm; > + localtime_r(&now_seconds, &tm); 4. Wtf? 2 system calls just to get the current time? Can you use just one? Why can't you use tv.tv_sec which you got above? > + now->offset = tm.tm_gmtoff / 60; > +} > + > +char * > +datetime_asctime(const struct datetime *date) > +{ > + struct tm *p_tm = datetime_to_tm(date); > + return asctime(p_tm); 5. You can't return a global buffer. You need to write the result into a local one passed in the arguments. AFAIS, there are functions with _r suffix which should help. > +} > + > +char * > +datetime_ctime(const struct datetime *date) > +{ > + time_t time = date->secs; > + return ctime(&time); > +} > + > +size_t > +datetime_strftime(const struct datetime *date, const char *fmt, char *buf, > + uint32_t len) > +{ > + struct tm *p_tm = datetime_to_tm(date); > + return strftime(buf, len, fmt, p_tm); > +} > diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h > new file mode 100644 > index 000000000..6699b31d3 > --- /dev/null > +++ b/src/lib/core/datetime.h > @@ -0,0 +1,94 @@ > +#pragma once > +/* > + * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include > +#include > +#include > +#include "c-dt/dt.h" > + > +#if defined(__cplusplus) > +extern "C" > +{ > +#endif /* defined(__cplusplus) */ > + > +#ifndef SECS_PER_DAY > +#define SECS_PER_DAY 86400 6. You have a tab here after the name. Please, use whitespace. > +#define DT_EPOCH_1970_OFFSET 719163 > +#endif > diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua > new file mode 100644 > index 000000000..a562b6872 > --- /dev/null > +++ b/src/lua/datetime.lua > @@ -0,0 +1,638 @@ > +local ffi = require('ffi') > + > +ffi.cdef [[ > + > + /* > + `c-dt` library functions handles properly both positive and negative `dt` > + values, where `dt` is a number of dates since Rata Die date (0001-01-01). > + > + For better compactness of our typical data in MessagePack stream we shift > + root of our time to the Unix Epoch date (1970-01-01), thus our 0 is > + actually dt = 719163. > + > + So here is a simple formula how convert our epoch-based seconds to dt values > + dt = (secs / 86400) + 719163 > + Where 719163 is an offset of Unix Epoch (1970-01-01) since Rata Die > + (0001-01-01) in dates. > + > + */ > + typedef int dt_t; > + > + // dt_core.h > + typedef enum { > + DT_MON = 1, > + DT_MONDAY = 1, > + DT_TUE = 2, > + DT_TUESDAY = 2, > + DT_WED = 3, > + DT_WEDNESDAY = 3, > + DT_THU = 4, > + DT_THURSDAY = 4, > + DT_FRI = 5, > + DT_FRIDAY = 5, > + DT_SAT = 6, > + DT_SATURDAY = 6, > + DT_SUN = 7, > + DT_SUNDAY = 7, > + } dt_dow_t; > + > + dt_t dt_from_rdn (int n); > + dt_t dt_from_yd (int y, int d); > + dt_t dt_from_ymd (int y, int m, int d); > + dt_t dt_from_yqd (int y, int q, int d); > + dt_t dt_from_ywd (int y, int w, int d); > + > + void dt_to_yd (dt_t dt, int *y, int *d); > + void dt_to_ymd (dt_t dt, int *y, int *m, int *d); > + void dt_to_yqd (dt_t dt, int *y, int *q, int *d); > + void dt_to_ywd (dt_t dt, int *y, int *w, int *d); > + > + int dt_rdn (dt_t dt); > + dt_dow_t dt_dow (dt_t dt); 7. dt_dow is not used in this file. The enum dt_dow_t either. The same for dt_from_rdn, dt_from_yd, dt_from_yqd, dt_from_ywd, dt_to_yd, dt_to_ymd, dt_to_yqd, dt_to_ywd. Please, check all the functions. I will return later with more comments. Need to go now.