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 2020/07/04 13:11:16 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1365: arch: Move the up_assert declaration from assert.h to arch.h

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


   and change the 1st argument from "const uint8_t *" to "const char *"
   
   ## Summary
   Ensure all arch function is declared inside nuttx/arch.h
   
   ## Impact
   No functionality change.
   
   ## Testing
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#discussion_r456132023



##########
File path: libs/libc/stdlib/lib_assert.c
##########
@@ -0,0 +1,38 @@
+/****************************************************************************
+ * libs/libc/stdlib/lib_assert.c

Review comment:
       Is this the right place for this function?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 merged pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1365: arch: up_assert shouldn't call exit directly

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


   @Ouss4 could you look at this patch is in the good shape?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1365: arch: up_assert shouldn't call exit directly

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


   @patacongo this patch remove exit call from up_assert which is part of https://github.com/apache/incubator-nuttx/issues/1263.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1365: arch: up_assert shouldn't call exit directly

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


   In the furture, exit forbid call from kernel directly because the most clean up step will move to userspace in exit after https://github.com/apache/incubator-nuttx/issues/1263.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#discussion_r456557360



##########
File path: libs/libc/stdlib/lib_assert.c
##########
@@ -0,0 +1,38 @@
+/****************************************************************************
+ * libs/libc/stdlib/lib_assert.c

Review comment:
       Done, move to libs/libc/assert




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1365: arch: up_assert shouldn't call exit directly

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


   @yamt 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1365: arch: up_assert shouldn't call exit directly

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


   > @xiaoxiang781216 Is there more in the PR that you have suggested? i.e. The removal of CONFIG_HAVE_FILENAME
   
   Yes, here https://github.com/apache/incubator-nuttx-apps/pull/318.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] davids5 commented on pull request #1365: arch: Move the up_assert declaration from assert.h to arch.h

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#issuecomment-653779983


   @xiaoxiang781216 Is there more in the PR that you have suggested? i.e. The removal of CONFIG_HAVE_FILENAME


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#issuecomment-654572542


   is abort() supposed to be callable from kernel?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#discussion_r456185863



##########
File path: libs/libc/stdlib/lib_assert.c
##########
@@ -0,0 +1,38 @@
+/****************************************************************************
+ * libs/libc/stdlib/lib_assert.c

Review comment:
       Do you have a better location? I select this location because lib_abort.c put here too. Or you like we create a new folder libs/libc/assert like other libc implementation.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#discussion_r456375529



##########
File path: libs/libc/stdlib/lib_assert.c
##########
@@ -0,0 +1,38 @@
+/****************************************************************************
+ * libs/libc/stdlib/lib_assert.c

Review comment:
       `abort()` is part of stdlib so it should be there.  I don't know about `_assert()`.  I honestly don't know about a better location.  Maybe as you suggested a new folder under libc?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1365: arch: up_assert shouldn't call exit directly

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1365:
URL: https://github.com/apache/incubator-nuttx/pull/1365#issuecomment-654581883


   In the furture, exit forbid call from kernel directly because the most clean up step will move to userspace in exit after https://github.com/apache/incubator-nuttx/issues/1263. But anyway, I will change to exit like before.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org