You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "xiaoxiang781216 (via GitHub)" <gi...@apache.org> on 2023/03/23 04:31:37 UTC
[GitHub] [nuttx] xiaoxiang781216 opened a new pull request, #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
xiaoxiang781216 opened a new pull request, #8878:
URL: https://github.com/apache/nuttx/pull/8878
## Summary
Fix error: 'NULL' undeclared if user include assert.h only
## Impact
Minor
## Testing
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] [nuttx] pkarashchenko commented on a diff in pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1146794275
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
Ok. I just checked. The parameter values are not checked. I was thinking that `-Wint-conversion` will catch it, but seems it handles only assignment cases (and passing a parameter is a kind of indirect assignment, so strange why this is not caught).
--
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] pkarashchenko commented on a diff in pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1146023104
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
Have you tested pure assert.h including on 64bit targets?
--
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] pkarashchenko commented on a diff in pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1146794275
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
Ok. I just checked. The parameter values are not checked. I was thinking that `-Wint-conversion` will catch it, but seems it handles only direct assignment cases (and passing a parameter is a kind of indirect assignment, so strange why this is not caught).
--
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] pkarashchenko commented on a diff in pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1145794407
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
Will this not lead to an error that integer is passed where the pointer is expected?
--
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] patacongo merged pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo merged PR #8878:
URL: https://github.com/apache/nuttx/pull/8878
--
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] pkarashchenko commented on a diff in pull request #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1146023104
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
Have you tested pure assert.h inclusion on 64bit targets?
--
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 #8878: assert.h: Replace NULL with 0 to avoid including stddef.h
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8878:
URL: https://github.com/apache/nuttx/pull/8878#discussion_r1145952452
##########
include/assert.h:
##########
@@ -66,20 +66,20 @@
} \
while (0)
#else
-# define ASSERT(f) \
- do \
- { \
- if (predict_false(!(f))) \
- __assert(__FILE__, __LINE__, NULL); \
- } \
+# define ASSERT(f) \
+ do \
+ { \
+ if (predict_false(!(f))) \
+ __assert(__FILE__, __LINE__, 0); \
Review Comment:
it's valid to pass 0 to pointer argument.
--
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