You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/08/04 12:22:21 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request, #6783: libc/localtime: fix ats over time_t range
Donny9 opened a new pull request, #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783
## Summary
libc/localtime: fix ats over time_t range
Fix time error for "set TZ :Pacific/Honolulu".
Signed-off-by: Jiuzhu Dong <do...@xiaomi.com>
## Impact
tzset correct
## Testing
Local test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r997943313
##########
libs/libc/time/lib_localtime.c:
##########
@@ -303,6 +303,12 @@ struct rule_s
int_fast32_t r_time; /* transition time of rule */
};
+struct rsem_s
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937783089
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
> how int64 value can be less than `INT64_MIN`? I mean what are the values to get this condition evaluated to `true`?
Refs to https://github.com/eggert/tz/blob/main/localtime.c#L518. Looks like the check can be removed and using at instead of attime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938390359
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
> I'm fine if conversion is needed. What I'm highlighting that some comparisons become not valid if we promote to 64bit
The latest patch replaces where only ats needs int64_t, doesn't affect other parts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937763289
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
It would make sense if `g_min_timet` is `INT32_MIN`, but not with `INT64_MIN`. Let me check the max condition as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937762094
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
I think even `<=` makes no sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937739405
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
how int64 value can be less than `INT64_MIN`? I mean what are the values to get this condition evaluated to `true`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937759615
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
@Donny9 probably intended `<=` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938714137
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
: 0);
-static const time_t g_max_timet =
- (TYPE_SIGNED(time_t)
- ? - (~ 0 < 0) - ((time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1))
+static const int_fast64_t g_max_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? - (~ 0 < 0) - ((int_fast64_t)1 <<
Review Comment:
Do we need to cast 0 in `~ 0` to `int_fast64_t`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998799660
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
+{
+ FAR char const *abbr = &sp->chars[sp->ttis[i].tt_desigidx];
+
+ /* memcmp is likely faster than strcmp, and is safe due to CHARS_EXTRA. */
+
+ return memcmp(abbr, UNSPEC, sizeof UNSPEC) == 0;
+}
+
static int_fast32_t detzcode(FAR const char *codep)
{
int_fast32_t result;
+ int_fast32_t one = 1;
+ int_fast32_t halfmaxval = one << (32 - 2);
+ int_fast32_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast32_t minval = -1 - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 4; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 4; ++i)
{
result = (result << 8) | (codep[i] & 0xff);
}
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0;
+ result += minval;
+ }
+
return result;
}
static int_fast64_t detzcode64(FAR const char *codep)
{
int_fast64_t result;
+ int_fast64_t one = 1;
+ int_fast64_t halfmaxval = one << (64 - 2);
+ int_fast64_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast64_t minval = -TWOS_COMPLEMENT(int_fast64_t) - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 8; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 8; ++i)
{
- result = (result * 256) | (codep[i] & 0xff);
+ result = (result << 8) | (codep[i] & 0xff);
+ }
+
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast64_t) && result != 0;
+ result += minval;
}
return result;
}
+static void scrub_abbrs(struct state_s *sp)
+{
+ int i;
+
+ /* First, replace bogus characters. */
+
+ for (i = 0; i < sp->charcnt; ++i)
+ {
+ if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
+ {
+ sp->chars[i] = TZ_ABBR_ERR_CHAR;
+ }
+ }
+
+ /* Second, truncate long abbreviations. */
+
+ for (i = 0; i < sp->typecnt; ++i)
+ {
+ FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
- }
-
- /* Finally, scrub the abbreviations. First, replace bogus characters. */
-
- for (i = 0; i < sp->charcnt; ++i)
- {
- if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
- {
- sp->chars[i] = TZ_ABBR_ERR_CHAR;
- }
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
+}
- /* Second, truncate long abbreviations. */
+static int_fast32_t leapcorr(FAR struct state_s const *sp, time_t t)
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#issuecomment-1211654499
> @Donny9 can this is somehow related to an issue that I asked in https://lists.apache.org/thread/h4pnnv1b9hfoglwg6hp6jz12xpytff7r ?
I‘m not sure if busyloop is related to "struct tm", i feel like which condition is always satisfied, causing busyloop. Maybe realted to "sinse TYPE_SIGNED(int_fast64_t) is always true"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938835344
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
: 0);
-static const time_t g_max_timet =
- (TYPE_SIGNED(time_t)
- ? - (~ 0 < 0) - ((time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1))
+static const int_fast64_t g_max_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? - (~ 0 < 0) - ((int_fast64_t)1 <<
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r997911007
##########
libs/libc/time/lib_localtime.c:
##########
@@ -303,6 +303,12 @@ struct rule_s
int_fast32_t r_time; /* transition time of rule */
};
+struct rsem_s
Review Comment:
let's use rmutex_t directly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998796561
##########
libs/libc/time/lib_localtime.c:
##########
@@ -611,168 +700,229 @@ static int tzload(FAR const char *name,
}
nread = _NX_READ(fid, up->buf, sizeof up->buf);
- if (_NX_CLOSE(fid) < 0 || nread <= 0)
+ if (_NX_CLOSE(fid) < 0 || nread < tzheadsize)
{
goto oops;
}
for (stored = 4; stored <= 8; stored *= 2)
{
- int ttisstdcnt;
- int ttisgmtcnt;
- int timecnt;
-
- ttisstdcnt = (int)detzcode(up->tzhead.tzh_ttisstdcnt);
- ttisgmtcnt = (int)detzcode(up->tzhead.tzh_ttisgmtcnt);
- sp->leapcnt = (int)detzcode(up->tzhead.tzh_leapcnt);
- sp->timecnt = (int)detzcode(up->tzhead.tzh_timecnt);
- sp->typecnt = (int)detzcode(up->tzhead.tzh_typecnt);
- sp->charcnt = (int)detzcode(up->tzhead.tzh_charcnt);
-
- p = up->tzhead.tzh_charcnt + sizeof up->tzhead.tzh_charcnt;
- if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
- sp->typecnt <= 0 || sp->typecnt > TZ_MAX_TYPES ||
- sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
- sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
- (ttisstdcnt != sp->typecnt && ttisstdcnt != 0) ||
- (ttisgmtcnt != sp->typecnt && ttisgmtcnt != 0))
+ char version = up->tzhead.tzh_version[0];
+ int skip_datablock = stored == 4 && version;
+ int_fast32_t datablock_size;
+ int_fast32_t ttisstdcnt;
+ int_fast32_t ttisutcnt;
+ int_fast32_t leapcnt;
+ int_fast32_t timecnt;
+ int_fast32_t typecnt;
+ int_fast32_t charcnt;
+ int_fast64_t prevtr = -1;
+ int_fast32_t prevcorr;
+ FAR const char *p;
+
+ ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt);
+ ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt);
+ leapcnt = detzcode(up->tzhead.tzh_leapcnt);
+ timecnt = detzcode(up->tzhead.tzh_timecnt);
+ typecnt = detzcode(up->tzhead.tzh_typecnt);
+ charcnt = detzcode(up->tzhead.tzh_charcnt);
+ p = up->buf + tzheadsize;
+ if (leapcnt < 0 || leapcnt > TZ_MAX_LEAPS ||
+ typecnt < 0 || typecnt > TZ_MAX_TYPES ||
+ timecnt < 0 || timecnt > TZ_MAX_TIMES ||
+ charcnt < 0 || charcnt > TZ_MAX_CHARS ||
+ ttisstdcnt < 0 || ttisstdcnt > TZ_MAX_TYPES ||
+ ttisutcnt < 0 || ttisutcnt > TZ_MAX_TYPES)
{
goto oops;
}
- if (nread - (p - up->buf) < sp->timecnt * stored + /* ats */
- sp->timecnt + /* types */
- sp->typecnt * 6 + /* ttinfos */
- sp->charcnt + /* chars */
- sp->leapcnt * (stored + 4) + /* lsinfos */
- ttisstdcnt + /* ttisstds */
- ttisgmtcnt) /* ttisgmts */
+ datablock_size = (timecnt * stored /* ats */
+ + timecnt /* types */
+ + typecnt * 6 /* ttinfos */
+ + charcnt /* chars */
+ + leapcnt * (stored + 4) /* lsinfos */
+ + ttisstdcnt /* ttisstds */
+ + ttisutcnt); /* ttisuts */
+ if (nread - tzheadsize < datablock_size)
{
goto oops;
}
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
+ if (skip_datablock)
{
- int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
- if (sp->types[i])
+ p += datablock_size;
+ }
+ else
+ {
+ if (!((ttisstdcnt == typecnt || ttisstdcnt == 0) &&
+ (ttisutcnt == typecnt || ttisutcnt == 0)))
+ {
+ goto oops;
+ }
+
+ sp->leapcnt = leapcnt;
+ sp->timecnt = timecnt;
+ sp->typecnt = typecnt;
+ sp->charcnt = charcnt;
+
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
+ sp->types[i] = at <= TIME_T_MAX;
+ if (sp->types[i])
{
- /* Keep the earlier record, but tweak
- * it so that it starts with the
- * minimum time_t value.
- */
+ time_t attime = ((TYPE_SIGNED(time_t) ?
+ at < TIME_T_MIN : at < 0) ?
+ TIME_T_MIN : at);
+ if (timecnt && attime <= sp->ats[timecnt - 1])
+ {
+ if (attime < sp->ats[timecnt - 1])
+ {
+ goto oops;
+ }
+
+ sp->types[i - 1] = 0;
+ timecnt--;
+ }
- sp->types[i - 1] = 1;
- sp->ats[timecnt++] = g_min_timet;
+ sp->ats[timecnt++] = attime;
}
- sp->ats[timecnt++] = at;
+ p += stored;
}
- p += stored;
- }
-
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
- {
- unsigned char typ = *p++;
- if (sp->typecnt <= typ)
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- goto oops;
+ unsigned char typ = *p++;
Review Comment:
Don't need?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r942085289
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
: 0);
-static const time_t g_max_timet =
- (TYPE_SIGNED(time_t)
- ? - (~ 0 < 0) - ((time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1))
+static const int_fast64_t g_max_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? - (~ (int_fast64_t)0 < 0) - ((int_fast64_t)1 <<
+ (CHAR_BIT * sizeof(int_fast64_t) - 1))
Review Comment:
I think that we need to switch to a fixed values here since `int_fast64_t` is used. The initial implementation was relying on assumption that `time_t` could be either signed or unsigned. If we switch to a fixed type then all this calculations do not make sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998956470
##########
libs/libc/time/lib_localtime.c:
##########
@@ -611,168 +700,229 @@ static int tzload(FAR const char *name,
}
nread = _NX_READ(fid, up->buf, sizeof up->buf);
- if (_NX_CLOSE(fid) < 0 || nread <= 0)
+ if (_NX_CLOSE(fid) < 0 || nread < tzheadsize)
{
goto oops;
}
for (stored = 4; stored <= 8; stored *= 2)
{
- int ttisstdcnt;
- int ttisgmtcnt;
- int timecnt;
-
- ttisstdcnt = (int)detzcode(up->tzhead.tzh_ttisstdcnt);
- ttisgmtcnt = (int)detzcode(up->tzhead.tzh_ttisgmtcnt);
- sp->leapcnt = (int)detzcode(up->tzhead.tzh_leapcnt);
- sp->timecnt = (int)detzcode(up->tzhead.tzh_timecnt);
- sp->typecnt = (int)detzcode(up->tzhead.tzh_typecnt);
- sp->charcnt = (int)detzcode(up->tzhead.tzh_charcnt);
-
- p = up->tzhead.tzh_charcnt + sizeof up->tzhead.tzh_charcnt;
- if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
- sp->typecnt <= 0 || sp->typecnt > TZ_MAX_TYPES ||
- sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
- sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
- (ttisstdcnt != sp->typecnt && ttisstdcnt != 0) ||
- (ttisgmtcnt != sp->typecnt && ttisgmtcnt != 0))
+ char version = up->tzhead.tzh_version[0];
+ int skip_datablock = stored == 4 && version;
+ int_fast32_t datablock_size;
+ int_fast32_t ttisstdcnt;
+ int_fast32_t ttisutcnt;
+ int_fast32_t leapcnt;
+ int_fast32_t timecnt;
+ int_fast32_t typecnt;
+ int_fast32_t charcnt;
+ int_fast64_t prevtr = -1;
+ int_fast32_t prevcorr;
+ FAR const char *p;
+
+ ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt);
+ ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt);
+ leapcnt = detzcode(up->tzhead.tzh_leapcnt);
+ timecnt = detzcode(up->tzhead.tzh_timecnt);
+ typecnt = detzcode(up->tzhead.tzh_typecnt);
+ charcnt = detzcode(up->tzhead.tzh_charcnt);
+ p = up->buf + tzheadsize;
+ if (leapcnt < 0 || leapcnt > TZ_MAX_LEAPS ||
+ typecnt < 0 || typecnt > TZ_MAX_TYPES ||
+ timecnt < 0 || timecnt > TZ_MAX_TIMES ||
+ charcnt < 0 || charcnt > TZ_MAX_CHARS ||
+ ttisstdcnt < 0 || ttisstdcnt > TZ_MAX_TYPES ||
+ ttisutcnt < 0 || ttisutcnt > TZ_MAX_TYPES)
{
goto oops;
}
- if (nread - (p - up->buf) < sp->timecnt * stored + /* ats */
- sp->timecnt + /* types */
- sp->typecnt * 6 + /* ttinfos */
- sp->charcnt + /* chars */
- sp->leapcnt * (stored + 4) + /* lsinfos */
- ttisstdcnt + /* ttisstds */
- ttisgmtcnt) /* ttisgmts */
+ datablock_size = (timecnt * stored /* ats */
+ + timecnt /* types */
+ + typecnt * 6 /* ttinfos */
+ + charcnt /* chars */
+ + leapcnt * (stored + 4) /* lsinfos */
+ + ttisstdcnt /* ttisstds */
+ + ttisutcnt); /* ttisuts */
+ if (nread - tzheadsize < datablock_size)
{
goto oops;
}
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
+ if (skip_datablock)
{
- int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
- if (sp->types[i])
+ p += datablock_size;
+ }
+ else
+ {
+ if (!((ttisstdcnt == typecnt || ttisstdcnt == 0) &&
+ (ttisutcnt == typecnt || ttisutcnt == 0)))
+ {
+ goto oops;
+ }
+
+ sp->leapcnt = leapcnt;
+ sp->timecnt = timecnt;
+ sp->typecnt = typecnt;
+ sp->charcnt = charcnt;
+
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
+ sp->types[i] = at <= TIME_T_MAX;
+ if (sp->types[i])
{
- /* Keep the earlier record, but tweak
- * it so that it starts with the
- * minimum time_t value.
- */
+ time_t attime = ((TYPE_SIGNED(time_t) ?
+ at < TIME_T_MIN : at < 0) ?
+ TIME_T_MIN : at);
+ if (timecnt && attime <= sp->ats[timecnt - 1])
+ {
+ if (attime < sp->ats[timecnt - 1])
+ {
+ goto oops;
+ }
+
+ sp->types[i - 1] = 0;
+ timecnt--;
+ }
- sp->types[i - 1] = 1;
- sp->ats[timecnt++] = g_min_timet;
+ sp->ats[timecnt++] = attime;
}
- sp->ats[timecnt++] = at;
+ p += stored;
}
- p += stored;
- }
-
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
- {
- unsigned char typ = *p++;
- if (sp->typecnt <= typ)
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- goto oops;
+ unsigned char typ = *p++;
Review Comment:
Yes. My bad
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998800066
##########
libs/libc/time/lib_localtime.c:
##########
@@ -611,168 +700,229 @@ static int tzload(FAR const char *name,
}
nread = _NX_READ(fid, up->buf, sizeof up->buf);
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -794,62 +944,123 @@ static int tzload(FAR const char *name,
sp->typecnt + 2 <= TZ_MAX_TYPES)
{
FAR struct state_s *ts = &lsp->u.st;
- int result;
up->buf[nread - 1] = '\0';
- result = tzparse(&up->buf[1], ts, FALSE);
- if (result == 0 && ts->typecnt == 2 &&
- sp->charcnt + ts->charcnt <= TZ_MAX_CHARS)
- {
- for (i = 0; i < 2; ++i)
- {
- ts->ttis[i].tt_abbrind += sp->charcnt;
- }
+ if (tzparse(&up->buf[1], ts, sp) == 0)
+ {
+ /* Attempt to reuse existing abbreviations.
+ * Without this, America/Anchorage would be right on
+ * the edge after 2037 when TZ_MAX_CHARS is 50, as
+ * sp->charcnt equals 40 (for LMT AST AWT APT AHST
+ * AHDT YST AKDT AKST) and ts->charcnt equals 10
+ * (for AKST AKDT). Reusing means sp->charcnt can
+ * stay 40 in this example.
+ */
- for (i = 0; i < ts->charcnt; ++i)
+ int gotabbr = 0;
+ int charcnt = sp->charcnt;
+ for (i = 0; i < ts->typecnt; i++)
{
- sp->chars[sp->charcnt++] = ts->chars[i];
- }
+ char *tsabbr = ts->chars + ts->ttis[i].tt_desigidx;
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -1768,13 +2053,14 @@ static FAR struct tm *gmtsub(FAR const time_t *timep,
if (!g_gmt_isset)
{
#ifndef __KERNEL__
- if (up_interrupt_context())
+ if (up_interrupt_context() || (sched_idletask() && OSINIT_IDLELOOP()))
{
return NULL;
}
#endif
- nxmutex_lock(&g_gmt_lock);
+ nxrmutex_lock(&g_gmt_lock);
+
if (!g_gmt_isset)
{
g_gmt_ptr = lib_malloc(sizeof *g_gmt_ptr);
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937761661
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
I think that building with `-Wall` and `-Wextra` would give a compiler warning for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938396437
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
The code in https://github.com/eggert/tz/blob/main/localtime.c can work correctly only when time_t is 64bits in the internal computation. Since NuttX define time_t as uint32_t, we need modify the internal function/data to int_fast64_t.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938511912
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
Review Comment:
CI report Error:
Error: time/lib_localtime.c:312:24: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
? (int_fast64_t)-1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
~~~~~~~~~~~~~~~~ ^
Error: time/lib_localtime.c:317:39: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
? - (~ 0 < 0) - ((int_fast64_t)-1 <<
~~~~~~~~~~~~~~~~ ^
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998800194
##########
libs/libc/time/lib_localtime.c:
##########
@@ -1785,21 +2071,27 @@ static FAR struct tm *gmtsub(FAR const time_t *timep,
}
}
- nxmutex_unlock(&g_gmt_lock);
+ nxrmutex_unlock(&g_gmt_lock);
}
- tmp->tm_zone = GMT;
+ tmp->tm_zone = ((char *)(offset ? g_wildabbr :
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#issuecomment-1211627055
> I hope to find some time end of week to get more precise look into this.
> Maybe you can describe some sample values that are failing currently, so I can focus on that.
@pkarashchenko Based on the modification of pr, i found it maybe caused busyloop at line 2320.
```
[01/00 00:00:00.000000] [ 0] [ap] clock_basetime, line 89
[01/00 00:00:00.000000] [ 0] [ap] clock_basetime, line 97
[01/00 00:00:00.000000] [ 0] [ap] timegm, line 2679
[01/00 00:00:00.000000] [ 0] [ap] time1, line 2479
[01/00 00:00:00.000000] [ 0] [ap] time1, line 2486
[01/00 00:00:00.000000] [ 0] [ap] time1, line 2492
[01/00 00:00:00.000000] [ 0] [ap] time2, line 2456
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2150
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2161
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2172
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2179
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2189
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2200
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2211
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2231
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2237
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2245
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2273
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2293
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
[01/00 00:00:00.000000] [ 0] [ap] time2sub, line 2320
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998655570
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
Review Comment:
```suggestion
static int ttunspecified(FAR const struct state_s *sp, int i)
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
- }
-
- /* Finally, scrub the abbreviations. First, replace bogus characters. */
-
- for (i = 0; i < sp->charcnt; ++i)
- {
- if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
- {
- sp->chars[i] = TZ_ABBR_ERR_CHAR;
- }
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
+}
- /* Second, truncate long abbreviations. */
+static int_fast32_t leapcorr(FAR struct state_s const *sp, time_t t)
Review Comment:
```suggestion
static int_fast32_t leapcorr(FAR const struct state_s *sp, time_t t)
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
- }
-
- /* Finally, scrub the abbreviations. First, replace bogus characters. */
-
- for (i = 0; i < sp->charcnt; ++i)
- {
- if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
- {
- sp->chars[i] = TZ_ABBR_ERR_CHAR;
- }
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
+}
- /* Second, truncate long abbreviations. */
+static int_fast32_t leapcorr(FAR struct state_s const *sp, time_t t)
+{
+ FAR struct lsinfo_s const *lp;
Review Comment:
```suggestion
FAR const struct lsinfo_s *lp;
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
+ size_t namelen = strlen(name);
+ char const tzdirslash[sizeof TZDIR] = TZDIR "/";
+
+ if (sizeof fullname - sizeof tzdirslash <= namelen)
Review Comment:
```suggestion
if (sizeof(fullname) - sizeof(tzdirslash) <= namelen)
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
Review Comment:
```suggestion
FAR const struct ttinfo_s * const ttisp = &sp->ttis[i];
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -794,62 +944,123 @@ static int tzload(FAR const char *name,
sp->typecnt + 2 <= TZ_MAX_TYPES)
{
FAR struct state_s *ts = &lsp->u.st;
- int result;
up->buf[nread - 1] = '\0';
- result = tzparse(&up->buf[1], ts, FALSE);
- if (result == 0 && ts->typecnt == 2 &&
- sp->charcnt + ts->charcnt <= TZ_MAX_CHARS)
- {
- for (i = 0; i < 2; ++i)
- {
- ts->ttis[i].tt_abbrind += sp->charcnt;
- }
+ if (tzparse(&up->buf[1], ts, sp) == 0)
+ {
+ /* Attempt to reuse existing abbreviations.
+ * Without this, America/Anchorage would be right on
+ * the edge after 2037 when TZ_MAX_CHARS is 50, as
+ * sp->charcnt equals 40 (for LMT AST AWT APT AHST
+ * AHDT YST AKDT AKST) and ts->charcnt equals 10
+ * (for AKST AKDT). Reusing means sp->charcnt can
+ * stay 40 in this example.
+ */
- for (i = 0; i < ts->charcnt; ++i)
+ int gotabbr = 0;
+ int charcnt = sp->charcnt;
+ for (i = 0; i < ts->typecnt; i++)
{
- sp->chars[sp->charcnt++] = ts->chars[i];
- }
+ char *tsabbr = ts->chars + ts->ttis[i].tt_desigidx;
Review Comment:
```suggestion
FAR char *tsabbr = ts->chars + ts->ttis[i].tt_desigidx;
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -611,168 +700,229 @@ static int tzload(FAR const char *name,
}
nread = _NX_READ(fid, up->buf, sizeof up->buf);
- if (_NX_CLOSE(fid) < 0 || nread <= 0)
+ if (_NX_CLOSE(fid) < 0 || nread < tzheadsize)
{
goto oops;
}
for (stored = 4; stored <= 8; stored *= 2)
{
- int ttisstdcnt;
- int ttisgmtcnt;
- int timecnt;
-
- ttisstdcnt = (int)detzcode(up->tzhead.tzh_ttisstdcnt);
- ttisgmtcnt = (int)detzcode(up->tzhead.tzh_ttisgmtcnt);
- sp->leapcnt = (int)detzcode(up->tzhead.tzh_leapcnt);
- sp->timecnt = (int)detzcode(up->tzhead.tzh_timecnt);
- sp->typecnt = (int)detzcode(up->tzhead.tzh_typecnt);
- sp->charcnt = (int)detzcode(up->tzhead.tzh_charcnt);
-
- p = up->tzhead.tzh_charcnt + sizeof up->tzhead.tzh_charcnt;
- if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
- sp->typecnt <= 0 || sp->typecnt > TZ_MAX_TYPES ||
- sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
- sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
- (ttisstdcnt != sp->typecnt && ttisstdcnt != 0) ||
- (ttisgmtcnt != sp->typecnt && ttisgmtcnt != 0))
+ char version = up->tzhead.tzh_version[0];
+ int skip_datablock = stored == 4 && version;
+ int_fast32_t datablock_size;
+ int_fast32_t ttisstdcnt;
+ int_fast32_t ttisutcnt;
+ int_fast32_t leapcnt;
+ int_fast32_t timecnt;
+ int_fast32_t typecnt;
+ int_fast32_t charcnt;
+ int_fast64_t prevtr = -1;
+ int_fast32_t prevcorr;
+ FAR const char *p;
+
+ ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt);
+ ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt);
+ leapcnt = detzcode(up->tzhead.tzh_leapcnt);
+ timecnt = detzcode(up->tzhead.tzh_timecnt);
+ typecnt = detzcode(up->tzhead.tzh_typecnt);
+ charcnt = detzcode(up->tzhead.tzh_charcnt);
+ p = up->buf + tzheadsize;
+ if (leapcnt < 0 || leapcnt > TZ_MAX_LEAPS ||
+ typecnt < 0 || typecnt > TZ_MAX_TYPES ||
+ timecnt < 0 || timecnt > TZ_MAX_TIMES ||
+ charcnt < 0 || charcnt > TZ_MAX_CHARS ||
+ ttisstdcnt < 0 || ttisstdcnt > TZ_MAX_TYPES ||
+ ttisutcnt < 0 || ttisutcnt > TZ_MAX_TYPES)
{
goto oops;
}
- if (nread - (p - up->buf) < sp->timecnt * stored + /* ats */
- sp->timecnt + /* types */
- sp->typecnt * 6 + /* ttinfos */
- sp->charcnt + /* chars */
- sp->leapcnt * (stored + 4) + /* lsinfos */
- ttisstdcnt + /* ttisstds */
- ttisgmtcnt) /* ttisgmts */
+ datablock_size = (timecnt * stored /* ats */
+ + timecnt /* types */
+ + typecnt * 6 /* ttinfos */
+ + charcnt /* chars */
+ + leapcnt * (stored + 4) /* lsinfos */
+ + ttisstdcnt /* ttisstds */
+ + ttisutcnt); /* ttisuts */
+ if (nread - tzheadsize < datablock_size)
{
goto oops;
}
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
+ if (skip_datablock)
{
- int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
- if (sp->types[i])
+ p += datablock_size;
+ }
+ else
+ {
+ if (!((ttisstdcnt == typecnt || ttisstdcnt == 0) &&
+ (ttisutcnt == typecnt || ttisutcnt == 0)))
+ {
+ goto oops;
+ }
+
+ sp->leapcnt = leapcnt;
+ sp->timecnt = timecnt;
+ sp->typecnt = typecnt;
+ sp->charcnt = charcnt;
+
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
+ sp->types[i] = at <= TIME_T_MAX;
+ if (sp->types[i])
{
- /* Keep the earlier record, but tweak
- * it so that it starts with the
- * minimum time_t value.
- */
+ time_t attime = ((TYPE_SIGNED(time_t) ?
+ at < TIME_T_MIN : at < 0) ?
+ TIME_T_MIN : at);
+ if (timecnt && attime <= sp->ats[timecnt - 1])
+ {
+ if (attime < sp->ats[timecnt - 1])
+ {
+ goto oops;
+ }
+
+ sp->types[i - 1] = 0;
+ timecnt--;
+ }
- sp->types[i - 1] = 1;
- sp->ats[timecnt++] = g_min_timet;
+ sp->ats[timecnt++] = attime;
}
- sp->ats[timecnt++] = at;
+ p += stored;
}
- p += stored;
- }
-
- timecnt = 0;
- for (i = 0; i < sp->timecnt; ++i)
- {
- unsigned char typ = *p++;
- if (sp->typecnt <= typ)
+ timecnt = 0;
+ for (i = 0; i < sp->timecnt; ++i)
{
- goto oops;
+ unsigned char typ = *p++;
Review Comment:
```suggestion
FAR unsigned char typ = *p++;
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -1785,21 +2071,27 @@ static FAR struct tm *gmtsub(FAR const time_t *timep,
}
}
- nxmutex_unlock(&g_gmt_lock);
+ nxrmutex_unlock(&g_gmt_lock);
}
- tmp->tm_zone = GMT;
+ tmp->tm_zone = ((char *)(offset ? g_wildabbr :
Review Comment:
```suggestion
tmp->tm_zone = ((FAR char *)(offset ? g_wildabbr :
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
+{
+ FAR char const *abbr = &sp->chars[sp->ttis[i].tt_desigidx];
+
+ /* memcmp is likely faster than strcmp, and is safe due to CHARS_EXTRA. */
+
+ return memcmp(abbr, UNSPEC, sizeof UNSPEC) == 0;
+}
+
static int_fast32_t detzcode(FAR const char *codep)
{
int_fast32_t result;
+ int_fast32_t one = 1;
+ int_fast32_t halfmaxval = one << (32 - 2);
+ int_fast32_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast32_t minval = -1 - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 4; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 4; ++i)
{
result = (result << 8) | (codep[i] & 0xff);
}
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0;
+ result += minval;
+ }
+
return result;
}
static int_fast64_t detzcode64(FAR const char *codep)
{
int_fast64_t result;
+ int_fast64_t one = 1;
+ int_fast64_t halfmaxval = one << (64 - 2);
+ int_fast64_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast64_t minval = -TWOS_COMPLEMENT(int_fast64_t) - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 8; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 8; ++i)
{
- result = (result * 256) | (codep[i] & 0xff);
+ result = (result << 8) | (codep[i] & 0xff);
+ }
+
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast64_t) && result != 0;
+ result += minval;
}
return result;
}
+static void scrub_abbrs(struct state_s *sp)
+{
+ int i;
+
+ /* First, replace bogus characters. */
+
+ for (i = 0; i < sp->charcnt; ++i)
+ {
+ if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
+ {
+ sp->chars[i] = TZ_ABBR_ERR_CHAR;
+ }
+ }
+
+ /* Second, truncate long abbreviations. */
+
+ for (i = 0; i < sp->typecnt; ++i)
+ {
+ FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
+ FAR char *cp = &sp->chars[ttisp->tt_desigidx];
+
+ if (strlen(cp) > TZ_ABBR_MAX_LEN && strcmp(cp, GRANDPARENTED) != 0)
+ {
+ *(cp + TZ_ABBR_MAX_LEN) = '\0';
+ }
+ }
+}
+
static void settzname(void)
{
FAR struct state_s *const sp = g_lcl_ptr;
Review Comment:
```suggestion
FAR struct state_s * const sp = g_lcl_ptr;
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
Review Comment:
```suggestion
FAR const char *dot;
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
+ size_t namelen = strlen(name);
+ char const tzdirslash[sizeof TZDIR] = TZDIR "/";
Review Comment:
```suggestion
const char tzdirslash[sizeof TZDIR] = TZDIR "/";
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
+{
+ FAR char const *abbr = &sp->chars[sp->ttis[i].tt_desigidx];
+
+ /* memcmp is likely faster than strcmp, and is safe due to CHARS_EXTRA. */
+
+ return memcmp(abbr, UNSPEC, sizeof UNSPEC) == 0;
+}
+
static int_fast32_t detzcode(FAR const char *codep)
{
int_fast32_t result;
+ int_fast32_t one = 1;
+ int_fast32_t halfmaxval = one << (32 - 2);
+ int_fast32_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast32_t minval = -1 - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 4; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 4; ++i)
{
result = (result << 8) | (codep[i] & 0xff);
}
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0;
+ result += minval;
+ }
+
return result;
}
static int_fast64_t detzcode64(FAR const char *codep)
{
int_fast64_t result;
+ int_fast64_t one = 1;
+ int_fast64_t halfmaxval = one << (64 - 2);
+ int_fast64_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast64_t minval = -TWOS_COMPLEMENT(int_fast64_t) - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 8; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 8; ++i)
{
- result = (result * 256) | (codep[i] & 0xff);
+ result = (result << 8) | (codep[i] & 0xff);
+ }
+
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast64_t) && result != 0;
+ result += minval;
}
return result;
}
+static void scrub_abbrs(struct state_s *sp)
+{
+ int i;
+
+ /* First, replace bogus characters. */
+
+ for (i = 0; i < sp->charcnt; ++i)
+ {
+ if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
+ {
+ sp->chars[i] = TZ_ABBR_ERR_CHAR;
+ }
+ }
+
+ /* Second, truncate long abbreviations. */
+
+ for (i = 0; i < sp->typecnt; ++i)
+ {
+ FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
Review Comment:
```suggestion
FAR const struct ttinfo_s * const ttisp = &sp->ttis[i];
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
- }
-
- /* Finally, scrub the abbreviations. First, replace bogus characters. */
-
- for (i = 0; i < sp->charcnt; ++i)
- {
- if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
- {
- sp->chars[i] = TZ_ABBR_ERR_CHAR;
- }
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
+}
- /* Second, truncate long abbreviations. */
+static int_fast32_t leapcorr(FAR struct state_s const *sp, time_t t)
+{
+ FAR struct lsinfo_s const *lp;
+ int i;
- for (i = 0; i < sp->typecnt; ++i)
+ i = sp->leapcnt;
+ while (--i >= 0)
{
- FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- FAR char *cp = &sp->chars[ttisp->tt_abbrind];
-
- if (strlen(cp) > TZ_ABBR_MAX_LEN && strcmp(cp, GRANDPARENTED) != 0)
+ lp = &sp->lsis[i];
+ if (t >= lp->ls_trans)
{
- *(cp + TZ_ABBR_MAX_LEN) = '\0';
+ return lp->ls_corr;
}
}
-}
-
-static int differ_by_repeat(time_t t1, time_t t0)
-{
- if (TYPE_BIT(time_t) - TYPE_SIGNED(time_t) < SECSPERREPEAT_BITS)
- {
- return 0;
- }
- return t1 - t0 == SECSPERREPEAT;
+ return 0;
}
static int tzload(FAR const char *name,
FAR struct state_s *sp, int doextend)
{
- FAR const char *p;
int i;
int fid;
int stored;
- int nread;
+ ssize_t nread;
typedef union
{
struct tzhead_s tzhead;
char buf[2 * sizeof(struct tzhead_s) +
- 2 * sizeof *sp +
+ 2 * sizeof (struct state_s) +
Review Comment:
```suggestion
2 * sizeof(struct state_s) +
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
Review Comment:
```suggestion
FAR const struct ttinfo_s * const ttisp = &sp->ttis[sp->types[i]];
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -611,168 +700,229 @@ static int tzload(FAR const char *name,
}
nread = _NX_READ(fid, up->buf, sizeof up->buf);
Review Comment:
```suggestion
nread = _NX_READ(fid, up->buf, sizeof(up->buf));
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -1768,13 +2053,14 @@ static FAR struct tm *gmtsub(FAR const time_t *timep,
if (!g_gmt_isset)
{
#ifndef __KERNEL__
- if (up_interrupt_context())
+ if (up_interrupt_context() || (sched_idletask() && OSINIT_IDLELOOP()))
{
return NULL;
}
#endif
- nxmutex_lock(&g_gmt_lock);
+ nxrmutex_lock(&g_gmt_lock);
+
if (!g_gmt_isset)
{
g_gmt_ptr = lib_malloc(sizeof *g_gmt_ptr);
Review Comment:
```suggestion
g_gmt_ptr = lib_malloc(sizeof(*g_gmt_ptr));
```
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
+ size_t namelen = strlen(name);
+ char const tzdirslash[sizeof TZDIR] = TZDIR "/";
+
+ if (sizeof fullname - sizeof tzdirslash <= namelen)
{
goto oops;
}
- strcpy(fullname, p);
- strcat(fullname, "/");
- strcat(fullname, name);
+ /* Create a string "TZDIR/NAME". Using sprintf here
+ * would pull in stdio (and would fail if the
+ * resulting string length exceeded INT_MAX!).
+ */
+
+ memcpy(fullname, tzdirslash, sizeof tzdirslash);
+ strcpy(fullname + sizeof tzdirslash, name);
Review Comment:
```suggestion
memcpy(fullname, tzdirslash, sizeof(tzdirslash));
strcpy(fullname + sizeof(tzdirslash), name);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998799767
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
- }
-
- /* Finally, scrub the abbreviations. First, replace bogus characters. */
-
- for (i = 0; i < sp->charcnt; ++i)
- {
- if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
- {
- sp->chars[i] = TZ_ABBR_ERR_CHAR;
- }
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
+}
- /* Second, truncate long abbreviations. */
+static int_fast32_t leapcorr(FAR struct state_s const *sp, time_t t)
+{
+ FAR struct lsinfo_s const *lp;
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
+ size_t namelen = strlen(name);
+ char const tzdirslash[sizeof TZDIR] = TZDIR "/";
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -578,22 +653,36 @@ static int tzload(FAR const char *name,
doaccess = name[0] == '/';
if (!doaccess)
{
- p = TZDIR;
- if (p == NULL ||
- FILENAME_MAX <= strlen(p) + strlen(name))
+ char const *dot;
+ size_t namelen = strlen(name);
+ char const tzdirslash[sizeof TZDIR] = TZDIR "/";
+
+ if (sizeof fullname - sizeof tzdirslash <= namelen)
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998800429
##########
libs/libc/time/lib_localtime.c:
##########
@@ -1785,21 +2071,27 @@ static FAR struct tm *gmtsub(FAR const time_t *timep,
}
}
- nxmutex_unlock(&g_gmt_lock);
+ nxrmutex_unlock(&g_gmt_lock);
}
- tmp->tm_zone = GMT;
+ tmp->tm_zone = ((char *)(offset ? g_wildabbr :
Review Comment:
Thank you very much.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r942086350
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,7 +677,7 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
+ sp->types[i] = ((TYPE_SIGNED(int_fast64_t)
? g_min_timet <= at : 0 <= at) &&
at <= g_max_timet);
Review Comment:
```suggestion
sp->types[i] = (g_min_timet <= at &&
at <= g_max_timet);
```
sinse `TYPE_SIGNED(int_fast64_t)` is always `true`
##########
libs/libc/time/lib_localtime.c:
##########
@@ -2031,15 +2032,15 @@ static int increment_overflow32(FAR int_fast32_t *lp, int m)
return FALSE;
}
-static int increment_overflow_time(FAR time_t *tp, int_fast32_t j)
+static int increment_overflow_time(FAR int_fast64_t *tp, int_fast32_t j)
{
/* This is like
* 'if (! (g_min_timet <= *tp + j && *tp + j <= g_max_timet)) ...',
* except that it does the right thing even if *tp + j would overflow.
*/
if (!(j < 0
- ? (TYPE_SIGNED(time_t) ? g_min_timet - j <= *tp : -1 - j < *tp)
+ ? (TYPE_SIGNED(int_fast64_t) ? g_min_timet - j <= *tp : -1 - j < *tp)
Review Comment:
```suggestion
? g_min_timet - j <= *tp
```
sinse `TYPE_SIGNED(int_fast64_t)` is always `true`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#issuecomment-1211635834
@Donny9 can this is somehow related to an issue that I asked in https://lists.apache.org/thread/h4pnnv1b9hfoglwg6hp6jz12xpytff7r ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix some issues and update to consistent with mainline
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r998799917
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
- tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_desigidx];
}
for (i = 0; i < sp->timecnt; ++i)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[sp->types[i]];
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -464,64 +554,48 @@ static void settzname(void)
{
FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
Review Comment:
Done!
##########
libs/libc/time/lib_localtime.c:
##########
@@ -412,40 +423,119 @@ static int typesequiv(FAR const struct state_s *sp, int a, int b);
static int tzload(FAR const char *name, FAR struct state_s *sp,
int doextend);
static int tzparse(FAR const char *name, FAR struct state_s *sp,
- int lastditch);
+ FAR struct state_s *basep);
/****************************************************************************
* Private Functions
****************************************************************************/
+/* Initialize *S to a value based on UTOFF, ISDST, and DESIGIDX. */
+
+static void init_ttinfo(FAR struct ttinfo_s *s, int_fast32_t utoff,
+ bool isdst, int desigidx)
+{
+ s->tt_utoff = utoff;
+ s->tt_isdst = isdst;
+ s->tt_desigidx = desigidx;
+ s->tt_ttisstd = false;
+ s->tt_ttisut = false;
+}
+
+/* Return true if SP's time type I does not specify local time. */
+
+static int ttunspecified(FAR struct state_s const *sp, int i)
+{
+ FAR char const *abbr = &sp->chars[sp->ttis[i].tt_desigidx];
+
+ /* memcmp is likely faster than strcmp, and is safe due to CHARS_EXTRA. */
+
+ return memcmp(abbr, UNSPEC, sizeof UNSPEC) == 0;
+}
+
static int_fast32_t detzcode(FAR const char *codep)
{
int_fast32_t result;
+ int_fast32_t one = 1;
+ int_fast32_t halfmaxval = one << (32 - 2);
+ int_fast32_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast32_t minval = -1 - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 4; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 4; ++i)
{
result = (result << 8) | (codep[i] & 0xff);
}
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0;
+ result += minval;
+ }
+
return result;
}
static int_fast64_t detzcode64(FAR const char *codep)
{
int_fast64_t result;
+ int_fast64_t one = 1;
+ int_fast64_t halfmaxval = one << (64 - 2);
+ int_fast64_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast64_t minval = -TWOS_COMPLEMENT(int_fast64_t) - maxval;
int i;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 8; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 8; ++i)
{
- result = (result * 256) | (codep[i] & 0xff);
+ result = (result << 8) | (codep[i] & 0xff);
+ }
+
+ if (codep[0] & 0x80)
+ {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ * If the result would be minval - 1, return minval.
+ */
+
+ result -= !TWOS_COMPLEMENT(int_fast64_t) && result != 0;
+ result += minval;
}
return result;
}
+static void scrub_abbrs(struct state_s *sp)
+{
+ int i;
+
+ /* First, replace bogus characters. */
+
+ for (i = 0; i < sp->charcnt; ++i)
+ {
+ if (strchr(TZ_ABBR_CHAR_SET, sp->chars[i]) == NULL)
+ {
+ sp->chars[i] = TZ_ABBR_ERR_CHAR;
+ }
+ }
+
+ /* Second, truncate long abbreviations. */
+
+ for (i = 0; i < sp->typecnt; ++i)
+ {
+ FAR const struct ttinfo_s *const ttisp = &sp->ttis[i];
+ FAR char *cp = &sp->chars[ttisp->tt_desigidx];
+
+ if (strlen(cp) > TZ_ABBR_MAX_LEN && strcmp(cp, GRANDPARENTED) != 0)
+ {
+ *(cp + TZ_ABBR_MAX_LEN) = '\0';
+ }
+ }
+}
+
static void settzname(void)
{
FAR struct state_s *const sp = g_lcl_ptr;
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938006992
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
I'm fine if conversion is needed. What I'm highlighting that some comparisons become not valid if we promote to 64bit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r938501730
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
: 0);
-static const time_t g_max_timet =
- (TYPE_SIGNED(time_t)
- ? - (~ 0 < 0) - ((time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1))
+static const int_fast64_t g_max_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? - (~ 0 < 0) - ((int_fast64_t)1 <<
Review Comment:
why switched from `-1` to `1` here?
##########
libs/libc/time/lib_localtime.c:
##########
@@ -307,14 +307,15 @@ struct rule_s
/* The minimum and maximum finite time values. */
-static const time_t g_min_timet =
- (TYPE_SIGNED(time_t)
- ? (time_t)-1 << (CHAR_BIT * sizeof(time_t) - 1)
+static const int_fast64_t g_min_timet =
+ (TYPE_SIGNED(int_fast64_t)
+ ? (int_fast64_t)1 << (CHAR_BIT * sizeof(int_fast64_t) - 1)
Review Comment:
why switched from `-1` to `1` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937821141
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
Yes, but initial code was relying on `time_t` size. The NuttX use `time_t` as `uint32_t`, so MIN and MAX are for 32-bit unsigned type. But in NuttX implementation we switch to `int_fast64_t` calculations, so I do not see how we can keep referencing to the https://github.com/eggert/tz/blob/main/localtime.c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6783: libc/localtime: fix ats over time_t range
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6783:
URL: https://github.com/apache/incubator-nuttx/pull/6783#discussion_r937840747
##########
libs/libc/time/lib_localtime.c:
##########
@@ -676,23 +668,22 @@ static int tzload(FAR const char *name,
for (i = 0; i < sp->timecnt; ++i)
{
int_fast64_t at = stored == 4 ? detzcode(p) : detzcode64(p);
- sp->types[i] = ((TYPE_SIGNED(time_t)
- ? g_min_timet <= at : 0 <= at) &&
- at <= g_max_timet);
+ sp->types[i] = at <= g_max_timet;
if (sp->types[i])
{
- if (i && !timecnt && at != g_min_timet)
+ int_fast64_t attime = at < g_min_timet ? g_min_timet : at;
Review Comment:
> Yes, but initial code was relying on `time_t` size. The NuttX use `time_t` as `uint32_t`, so MIN and MAX are for 32-bit unsigned type. But in NuttX implementation we switch to `int_fast64_t` calculations, so I do not see how we can keep referencing to the https://github.com/eggert/tz/blob/main/localtime.c
Yes, For Nuttx in `time_t` as `uint32_t`:
```
the TYPE_SIGNED(time_t) is ture,
he g_min_timet is 0,
the g_max_timet is (time_t)-1.
```
For tzfile "Pacific/Honolulu", ats is `ats = {-2334101314, -1157283000, -1155436200, -880198200, -769395600, -765376200, -712150200,` it's not in the range of `uint32` and `int32` anyway. so need to convert int_fast64_t
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org