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 2021/10/19 03:08:28 UTC
[GitHub] [incubator-nuttx] anjiahao1 opened a new pull request #4693: time:follow POSIX use int64_t to define time_t
anjiahao1 opened a new pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693
## Summary
follow posix,use int64_t inside of time_t
## Impact
## Testing
dailytest
--
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] patacongo commented on a change in pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731871925
##########
File path: include/time.h
##########
@@ -102,7 +102,12 @@
/* Scalar types */
+#ifdef CONFIG_SYSTEM_TIME64
+typedef uint64_t time_t; /* Holds time in seconds */
+#else
typedef uint32_t time_t; /* Holds time in seconds */
+#endif
+
Review comment:
> So we add new CONFIG to specify the size of time_t?
Or, better, keep it 32-bits. Or, perhaps, have it depend only on CONFIG_HAVE_LONG_LONG
--
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] anjiahao1 commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946337112
> > > > follow posix,use int64_t inside of time_t
> > >
> > >
> > > which part of posix?
> >
> >
> > clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
> > if use a `tp->tv_sec = -1` before will return ok,now return EINVAL
>
> can you give me pointers to the relevant parts of the posix?
https://man7.org/linux/man-pages/man3/clock_settime.3.html
--
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] yamt commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946370843
> > > > as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
> > >
> > >
> > > ISO-C allow signed or unsigned time_t, but POSIX require the signed integer: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
> >
> >
> > the page says:
> > > time_t and clock_t shall be integer or real-floating types.
> >
> >
> > iirc the more recent versions of posix require it to be an integer. but i'm not aware of a version which requires signed.
>
> Here the integer ahould mean signed, since other place use unsigned integer in the same page.
no. the page actually uses three kinds.
* integer
* signed integer
* unsigned integer
>
> > > From this thread: https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
> >
> >
> > i tend to agree, while i have a vague concern about using 64-bit types for small devices.
>
> time_t is still int32_t if CONFIG_SYSTEM_TIME64 isnot defined.
if we need 32 bit version, it should be unsigned, as int32_t will wrap too soon.
i guess 64 bit version should be unsigned too because it should be consistent within nuttx.
--
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] yamt commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946330813
> follow posix,use int64_t inside of time_t
which part of posix?
--
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] patacongo commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946356929
This will break several architectures that do not support 64-bit long long types. This includes *all* of the ZiLOG architectures. This logic all needs to conditioned on CONFIG_HAVE_LONG_LONG.
--
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] anjiahao1 commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946334428
> > follow posix,use int64_t inside of time_t
>
> which part of posix?
clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
if use a `tp->tv_sec = -1`
before will return ok,now return EINVAL
--
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] patacongo commented on a change in pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731871925
##########
File path: include/time.h
##########
@@ -102,7 +102,12 @@
/* Scalar types */
+#ifdef CONFIG_SYSTEM_TIME64
+typedef uint64_t time_t; /* Holds time in seconds */
+#else
typedef uint32_t time_t; /* Holds time in seconds */
+#endif
+
Review comment:
> So we add new CONFIG to specify the size of time_t?
Or, better, keep it 32-bits.
--
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] yamt commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946341854
> > > > > follow posix,use int64_t inside of time_t
> > > >
> > > >
> > > > which part of posix?
> > >
> > >
> > > clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
> > > if use a `tp->tv_sec = -1` before will return ok,now return EINVAL
> >
> >
> > can you give me pointers to the relevant parts of the posix?
>
> https://man7.org/linux/man-pages/man3/clock_settime.3.html
isn't it linux-specific documentation?
do you mean you interpreted the following part to mean tv_sec should be signed?
```
EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is
outside the range [0..999,999,999].
```
--
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] patacongo commented on a change in pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731793229
##########
File path: include/sys/types.h
##########
@@ -230,8 +230,10 @@ typedef uint16_t sa_family_t;
#ifdef CONFIG_SYSTEM_TIME64
typedef uint64_t clock_t;
+typedef int64_t time_t;
#else
typedef uint32_t clock_t;
+typedef int32_t time_t;
#endif
Review comment:
>
>
> time.h include sys/types.h at the begin, so there is no real difference from the user perspective. putting here is to make all CONFIG_SYSTEM_TIME64 related public definition in the same place.
True.. the only difference is that one location complies with the requirements of POSIX and the other does not. POSIX are requires the time_t *not* be defined if time_t is not included.
--
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 pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946413371
> > > > > as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
> > > >
> > > >
> > > > ISO-C allow signed or unsigned time_t, but POSIX require the signed integer: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
> > >
> > >
> > > the page says:
> > > > time_t and clock_t shall be integer or real-floating types.
> > >
> > >
> > > iirc the more recent versions of posix require it to be an integer. but i'm not aware of a version which requires signed.
> >
> >
> > Here the integer ahould mean signed, since other place use unsigned integer in the same page.
>
> no. the page actually uses three kinds.
>
> * integer
> * signed integer
> * unsigned integer
>
You are right.
> > > > From this thread: https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
> > >
> > >
> > > i tend to agree, while i have a vague concern about using 64-bit types for small devices.
> >
> >
> > time_t is still int32_t if CONFIG_SYSTEM_TIME64 isnot defined.
>
> if we need 32 bit version, it should be unsigned, as int32_t will wrap too soon. i guess 64 bit version should be unsigned too because it should be consistent within nuttx.
Yes, int32_t will wrap at 2038(https://en.wikipedia.org/wiki/Year_2038_problem).
--
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] yamt commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946335244
> > > follow posix,use int64_t inside of time_t
> >
> >
> > which part of posix?
>
> clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
>
> if use a `tp->tv_sec = -1` before will return ok,now return EINVAL
can you give me pointers to the relevant parts of the posix?
--
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 pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946366697
> > > as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
> >
> >
> > ISO-C allow signed or unsigned time_t, but POSIX require the signed integer: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
>
> the page says:
>
> > time_t and clock_t shall be integer or real-floating types.
>
> iirc the more recent versions of posix require it to be an integer. but i'm not aware of a version which requires signed.
>
Here the integer ahould mean signed, since other place use unsigned integer in the same page.
> > From this thread: https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
>
> i tend to agree, while i have a vague concern about using 64-bit types for small devices.
time_t is still int32_t if CONFIG_SYSTEM_TIME64 isnot defined.
--
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 change in pull request #4693: time_t:add unsigned integer define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731826798
##########
File path: include/sys/types.h
##########
@@ -230,8 +230,10 @@ typedef uint16_t sa_family_t;
#ifdef CONFIG_SYSTEM_TIME64
typedef uint64_t clock_t;
+typedef int64_t time_t;
#else
typedef uint32_t clock_t;
+typedef int32_t time_t;
#endif
Review comment:
Ok, time_t is kept in the original location.
--
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 pull request #4693: time_t:add unsigned integer define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946686089
> > int64_t tyoedef is guarded by CONFIG_SYSTEM_TIME64, so it is not real issue.
>
> It is an issue because CONFIG_SYSTEM_TIME64 value is not effected by CONFIG_HAVE_LONG_LONG; the first is defined by the configuration, the second by compiler.h. nuttx/clock.h "fixes" any inconsistency. However, nuttx/clock.h that is not included by sys/types.h.
Move the CONFIG_HAVE_LONG_LONG check to sys/types.h
--
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 pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946353572
> as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
ISO-C allow signed or unsigned time_t, but POSIX require the signed integer:
https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
From this thread:
https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to
Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
--
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] anjiahao1 commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946345372
yse,i use `man p clock_settime` see
`EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is outside the range [0..999,999,999].`
--
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] anjiahao1 removed a comment on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 removed a comment on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946345372
yse,i use `man p clock_settime` see
`EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is outside the range [0..999,999,999].`
--
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 change in pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731866866
##########
File path: include/time.h
##########
@@ -102,7 +102,12 @@
/* Scalar types */
+#ifdef CONFIG_SYSTEM_TIME64
+typedef uint64_t time_t; /* Holds time in seconds */
+#else
typedef uint32_t time_t; /* Holds time in seconds */
+#endif
+
Review comment:
So we add new CONFIG to specify the size of time_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] yamt commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946357628
> > as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
>
> ISO-C allow signed or unsigned time_t, but POSIX require the signed integer: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
the page says:
> time_t and clock_t shall be integer or real-floating types.
iirc the more recent versions of posix require it to be an integer.
but i'm not aware of a version which requires unsigned.
> From this thread: https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
i tend to agree, while i have a vague concern about using 64-bit types for small devices.
--
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 pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946364664
> This will break several architectures that do not support 64-bit long long types. This includes _all_ of the ZiLOG architectures. This logic all needs to conditioned on CONFIG_HAVE_LONG_LONG.
int64_t tyoedef is guarded by CONFIG_SYSTEM_TIME64, so it is not real issue.
--
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 pull request #4693: time_t:add unsigned integer define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946687511
> if we need 32 bit version, it should be unsigned, as int32_t will wrap too soon. i guess 64 bit version should be unsigned too because it should be consistent within nuttx.
Ok, change to uint32_t/uint64_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] patacongo commented on pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946728343
> > if we need 32 bit version, it should be unsigned, as int32_t will wrap too soon. i guess 64 bit version should be unsigned too because it should be consistent within nuttx.
>
> Yes, int32_t will wrap at 2038(https://en.wikipedia.org/wiki/Year_2038_problem).
Negative time is never used in NuttX. Negative time could arise in applications as a consequence of arithmetic operations, but that would be a non-portable behavior of the application because POSIX permits unsigned time_t. For portability, applications should always cast to an appropriate time before performing signed operations.
With unsigned time_t, the range is doubled. Instead of 2038 (1970 + 68) then it should overflow at (1970 + 2*68) = 2,206. Prior to that date, the upper 32-bits of a 64-bit time_t will always be zero. So a 64-bit is totally wasteful until then. The effect is no just limited to time_t parameters, but to the size of all structures that include a time_t field.
--
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] anjiahao1 edited a comment on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 edited a comment on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946345549
> > > > > > follow posix,use int64_t inside of time_t
> > > > >
> > > > >
> > > > > which part of posix?
> > > >
> > > >
> > > > clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
> > > > if use a `tp->tv_sec = -1` before will return ok,now return EINVAL
> > >
> > >
> > > can you give me pointers to the relevant parts of the posix?
> >
> >
> > https://man7.org/linux/man-pages/man3/clock_settime.3.html
>
> isn't it linux-specific documentation?
>
> do you mean you interpreted the following part to mean tv_sec should be signed?
>
> ```
> EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is
> outside the range [0..999,999,999].
> ```
yse,i use `man p clock_settime `see
`EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is outside the range [0..999,999,999].`
--
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 change in pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731506736
##########
File path: include/sys/types.h
##########
@@ -230,8 +230,10 @@ typedef uint16_t sa_family_t;
#ifdef CONFIG_SYSTEM_TIME64
typedef uint64_t clock_t;
+typedef int64_t time_t;
#else
typedef uint32_t clock_t;
+typedef int32_t time_t;
#endif
Review comment:
time.h include sys/types.h at the begin, so there is no real difference from the user perspective. putting here is to make all CONFIG_SYSTEM_TIME64 related public definition in the same place.
--
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] patacongo commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946658415
> int64_t tyoedef is guarded by CONFIG_SYSTEM_TIME64, so it is not real issue.
It is an issue because CONFIG_SYSTEM_TIME64 value is not effected by CONFIG_HAVE_LONG_LONG; the first is defined by the configuration, the second by compiler.h. nuttx/clock.h "fixes" any inconsistency. However, nuttx/clock.h that is not included by sys/types.h.
--
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] anjiahao1 closed pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
anjiahao1 closed pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693
--
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] patacongo commented on a change in pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731862198
##########
File path: include/time.h
##########
@@ -102,7 +102,12 @@
/* Scalar types */
+#ifdef CONFIG_SYSTEM_TIME64
+typedef uint64_t time_t; /* Holds time in seconds */
+#else
typedef uint32_t time_t; /* Holds time in seconds */
+#endif
+
Review comment:
CONFIG_SYSTEM_TIME64 determines if the high precision system timer is 32- or 32-bits with. time_t is the number of seconds since the epoch. They are unrelated. I don't see why the width of time_t should should be side of effect and depend on the width of the high precision timer. Seems like a unnecessary, undesirable coupling.
--
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] patacongo commented on a change in pull request #4693: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731862198
##########
File path: include/time.h
##########
@@ -102,7 +102,12 @@
/* Scalar types */
+#ifdef CONFIG_SYSTEM_TIME64
+typedef uint64_t time_t; /* Holds time in seconds */
+#else
typedef uint32_t time_t; /* Holds time in seconds */
+#endif
+
Review comment:
CONFIG_SYSTEM_TIME64 determines if the high precision system timer is 32- or 32-bits with. time_t is the number of seconds since the epoch. They are unrelated. I don't see why the width of time_t should depend on the width of the high precision timer. Seems like a unnecessary, undesirable coupling.
--
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] juniskane commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946607463
> > iirc the more recent versions of posix require it to be an integer. but i'm not aware of a version which requires signed.
>
> Here the integer ahould mean signed, since other place use unsigned integer in the same page.
Most recent version here:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
says "time_t shall be an integer type". (This is marked as C extension, because ISO C also allows floating point time_t)
I fail to see how rest of page using unsigned integer could be relevant, as the technical term "integer type" has a precisely defined meaning in ISO 9899:1999 (C99 standard, incorporated to Open Group spec by reference) section 6.2.5, clause 17: "The type **char**, the signed and unsigned integer types, and the enumerated types are collectively called _integer types_. The integer and real floating types are collectively called _real types_."
I haven't looked inside this commit, but the claim that this is needed to "follow POSIX" in commit message's summary line is incorrect.
--
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] anjiahao1 commented on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946345549
> > > > > > follow posix,use int64_t inside of time_t
> > > > >
> > > > >
> > > > > which part of posix?
> > > >
> > > >
> > > > clock_settime(clockid_t clock_id, FAR const struct timespec *tp)
> > > > if use a `tp->tv_sec = -1` before will return ok,now return EINVAL
> > >
> > >
> > > can you give me pointers to the relevant parts of the posix?
> >
> >
> > https://man7.org/linux/man-pages/man3/clock_settime.3.html
>
> isn't it linux-specific documentation?
>
> do you mean you interpreted the following part to mean tv_sec should be signed?
>
> ```
> EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is
> outside the range [0..999,999,999].
> ```
yse,i use man p clock_settime see
EINVAL (clock_settime()): tp.tv_sec is negative or tp.tv_nsec is outside the range [0..999,999,999].
--
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] yamt edited a comment on pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#issuecomment-946357628
> > as far as i know, unsigned time_t is allowed by posix. this change requires more explanation than "follow posix".
>
> ISO-C allow signed or unsigned time_t, but POSIX require the signed integer: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html
the page says:
> time_t and clock_t shall be integer or real-floating types.
iirc the more recent versions of posix require it to be an integer.
but i'm not aware of a version which requires signed.
> From this thread: https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to Most Unix variant OS use signed integer represent time_t, so it's better to follow the common practice to improve the compability.
i tend to agree, while i have a vague concern about using 64-bit types for small devices.
--
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] patacongo commented on a change in pull request #4693: time:follow POSIX use int64_t to define time_t
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4693:
URL: https://github.com/apache/incubator-nuttx/pull/4693#discussion_r731484565
##########
File path: include/sys/types.h
##########
@@ -230,8 +230,10 @@ typedef uint16_t sa_family_t;
#ifdef CONFIG_SYSTEM_TIME64
typedef uint64_t clock_t;
+typedef int64_t time_t;
#else
typedef uint32_t clock_t;
+typedef int32_t time_t;
#endif
Review comment:
POISIX *requires* that time_t be defined in time.h: https://pubs.opengroup.org/onlinepubs/7908799/xsh/time.h.html
--
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