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/09/16 18:10:29 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

xiaoxiang781216 opened a new pull request, #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115

   ## Summary
   to handle 2038 problem correctly
   
   ## Impact
   The type of time_t
    
   ## Testing
   Pass CI
   


-- 
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 #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#discussion_r999669764


##########
include/time.h:
##########
@@ -98,7 +98,11 @@
 
 /* Scalar types */
 
+#ifdef CONFIG_SYSTEM_TIME64
+typedef int64_t   time_t;         /* Holds time in seconds */

Review Comment:
   most system typedef time_t to signed type(32bit or 64bit):
   https://stackoverflow.com/questions/471248/what-is-time-t-ultimately-a-typedef-to
   NuttX is a special one which define time_t to unsigned 32bit. I guess the reason is that the unsigned type could double the the range of representation.
   But 64bit has enough bit to represent the very very long range human being can expect, so I follow other OS convention to typedef time_t to signed 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] xiaoxiang781216 commented on pull request #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#issuecomment-1249752681

   Yes, let's fix the problem phase by phase.


-- 
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 #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#issuecomment-1250068505

   Wait for https://github.com/apache/incubator-nuttx/pull/6783, mark as draft.


-- 
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 #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#issuecomment-1249721734

   Seems not all parts of the system are ready 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] [nuttx] yamt commented on a diff in pull request #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
yamt commented on code in PR #7115:
URL: https://github.com/apache/nuttx/pull/7115#discussion_r1049254121


##########
include/time.h:
##########
@@ -98,7 +98,11 @@
 
 /* Scalar types */
 
+#ifdef CONFIG_SYSTEM_TIME64
+typedef int64_t   time_t;         /* Holds time in seconds */

Review Comment:
   as discussed in https://github.com/apache/nuttx/pull/4693
   consistency within nuttx outweighs consistency with other systems.
   can you consider to make it unsigned?



-- 
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7115:
URL: https://github.com/apache/nuttx/pull/7115#discussion_r1049277487


##########
include/time.h:
##########
@@ -98,7 +98,11 @@
 
 /* Scalar types */
 
+#ifdef CONFIG_SYSTEM_TIME64
+typedef int64_t   time_t;         /* Holds time in seconds */

Review Comment:
   It's fine to use unsigned, could you provide a patch?



-- 
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 merged pull request #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115


-- 
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 #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#discussion_r999623067


##########
include/time.h:
##########
@@ -98,7 +98,11 @@
 
 /* Scalar types */
 
+#ifdef CONFIG_SYSTEM_TIME64
+typedef int64_t   time_t;         /* Holds time in seconds */

Review Comment:
   @xiaoxiang781216 is this issue https://github.com/apache/incubator-nuttx/issues/3390 related to this PR?
   I mean that for 64 bit we are going with signed while with 32 bit with unsigned



-- 
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] [nuttx] yamt commented on pull request #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
yamt commented on PR #7115:
URL: https://github.com/apache/nuttx/pull/7115#issuecomment-1352612524

   this change still have the problems pointed out in the older PR.
   https://github.com/apache/nuttx/pull/4693#discussion_r731862198
   https://github.com/apache/nuttx/pull/4693#issuecomment-946687511
   


-- 
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 #7115: time: Typedef time_t to uint64_t if CONFIG_SYSTEM_TIME64 is defined

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7115:
URL: https://github.com/apache/incubator-nuttx/pull/7115#issuecomment-1283420100

   > Wait for #6783, mark as draft.
   
   @pkarashchenko this patch is ready now.


-- 
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