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/02/16 17:37:25 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
xiaoxiang781216 opened a new pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519
## Summary
```
Error: module/mod_insmod.c:203:3: error: 'strncpy' specified bound 16 equals destination size [-Werror=stringop-truncation]
203 | strncpy(modp->modname, modname, MODLIB_NAMEMAX);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
wqueue/kwork_thread.c: In function 'work_start_lowpri':
Error: wqueue/kwork_thread.c:212:22: error: '%lx' directive output may be truncated writing between 1 and 16 bytes into a region of size 14 [-Werror=format-truncation=]
212 | snprintf(args, 16, "0x%" PRIxPTR, (uintptr_t)wqueue);
local/local_sockif.c: In function 'local_getsockname':
Error: local/local_sockif.c:392:11: error: 'strncpy' specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
392 | strncpy(unaddr->sun_path, conn->lc_path, namelen);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
chip/esp32_wifi_utils.c: In function 'esp_wifi_scan_event_parse':
Error: chip/esp32_wifi_utils.c:373:37: error: argument to 'sizeof' in 'memset' call is the same expression as the destination; did you mean to dereference it? [-Werror=sizeof-pointer-memaccess]
memset(ap_list_buffer, 0x0, sizeof(ap_list_buffer));
^
stdio/lib_fputs.c: In function 'fputs':
Error: stdio/lib_fputs.c:99:9: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s == NULL || stream == NULL)
^
Error: stdio/lib_fputs.c:99:27: error: nonnull argument 'stream' compared to NULL [-Werror=nonnull-compare]
if (s == NULL || stream == NULL)
^
stdio/lib_vfprintf.c: In function 'vfprintf':
Error: stdio/lib_vfprintf.c:40:6: error: nonnull argument 'stream' compared to NULL [-Werror=nonnull-compare]
if (stream)
^
string/lib_strdup.c: In function 'strdup':
Error: string/lib_strdup.c:39:6: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s)
^
string/lib_strndup.c: In function 'strndup':
Error: string/lib_strndup.c:56:6: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s)
^
string/lib_strpbrk.c: In function 'strpbrk':
Error: string/lib_strpbrk.c:39:7: error: nonnull argument 'str' compared to NULL [-Werror=nonnull-compare]
if (!str || !charset)
^~~~
Error: string/lib_strpbrk.c:39:15: error: nonnull argument 'charset' compared to NULL [-Werror=nonnull-compare]
if (!str || !charset)
^~~~~~~~
string/lib_strrchr.c: In function 'strrchr':
Error: string/lib_strrchr.c:40:6: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s)
^
Error: time/lib_asctimer.c:73:50: error: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size between 0 and 12 [-Werror=format-truncation=]
snprintf(buf, 26, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
^~
time/lib_asctimer.c:73:21: note: directive argument in the range [-2147481748, 2147483647]
snprintf(buf, 26, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
time/lib_asctimer.c:73:3: note: 'snprintf' output between 17 and 68 bytes into a destination of size 26
snprintf(buf, 26, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
g_wday_name[tp->tm_wday], g_mon_name[tp->tm_mon],
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tp->tm_mday, tp->tm_hour, tp->tm_min, tp->tm_sec,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1900 + tp->tm_year);
~~~~~~~~~~~~~~~~~~~
```
## Impact
Compiler warning
## 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 change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810113074
##########
File path: libs/libc/string/lib_strpbrk.c
##########
@@ -33,15 +33,6 @@
#undef strpbrk /* See mm/README.txt */
FAR char *strpbrk(FAR const char *str, FAR const char *charset)
{
- /* Sanity checking */
-
-#ifdef CONFIG_DEBUG_FEATURES
- if (!str || !charset)
- {
- return NULL;
- }
-#endif
-
Review comment:
Fix this warning wihtout -fno-builtin:
```
string/lib_strpbrk.c: In function 'strpbrk':
Error: string/lib_strpbrk.c:39:7: error: nonnull argument 'str' compared to NULL [-Werror=nonnull-compare]
if (!str || !charset)
^~~~
Error: string/lib_strpbrk.c:39:15: error: nonnull argument 'charset' compared to NULL [-Werror=nonnull-compare]
if (!str || !charset)
^~~~~~~~
```
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r811133463
##########
File path: libs/libc/string/lib_strrchr.c
##########
@@ -37,15 +37,13 @@
#undef strrchr /* See mm/README.txt */
FAR char *strrchr(FAR const char *s, int c)
{
- if (s)
+ const char *p = &s[strlen(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] xiaoxiang781216 commented on a change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r811134387
##########
File path: libs/libc/string/lib_memchr.c
##########
@@ -49,17 +49,14 @@ FAR void *memchr(FAR const void *s, int c, size_t n)
{
FAR const unsigned char *p = (FAR const unsigned char *)s;
- if (s)
+ while (n--)
{
- while (n--)
+ if (*p == (unsigned char)c)
Review comment:
the code may want to avoid depend on whether char is extend as unsigned or signed.
--
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 change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r811128623
##########
File path: libs/libc/string/lib_strrchr.c
##########
@@ -37,15 +37,13 @@
#undef strrchr /* See mm/README.txt */
FAR char *strrchr(FAR const char *s, int c)
{
- if (s)
+ const char *p = &s[strlen(s)];
Review comment:
```suggestion
FAR const char *p = &s[strlen(s)];
```
##########
File path: libs/libc/string/lib_memchr.c
##########
@@ -49,17 +49,14 @@ FAR void *memchr(FAR const void *s, int c, size_t n)
{
FAR const unsigned char *p = (FAR const unsigned char *)s;
- if (s)
+ while (n--)
{
- while (n--)
+ if (*p == (unsigned char)c)
Review comment:
not related to this PR, but just curious why we are using `unsigned char` and not `char` 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] hartmannathan commented on a change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r809164741
##########
File path: libs/libc/string/lib_memchr.c
##########
@@ -49,17 +49,14 @@ FAR void *memchr(FAR const void *s, int c, size_t n)
{
FAR const unsigned char *p = (FAR const unsigned char *)s;
- if (s)
Review comment:
Do we really want to eliminate this NULL pointer check here?
If 's' is NULL, I feel it is safer to return NULL than to have undefined effects.
##########
File path: libs/libc/stdio/lib_fputs.c
##########
@@ -93,16 +93,6 @@ int fputs(FAR const IPTR char *s, FAR FILE *stream)
{
int nput;
- /* Make sure that a string was provided. */
-
-#ifdef CONFIG_DEBUG_FEATURES /* Most parameter checking is disabled if DEBUG is off */
- if (s == NULL || stream == NULL)
- {
- set_errno(EINVAL);
- return EOF;
- }
-#endif
-
Review comment:
Why eliminate the optional check?
Removing it makes this version of fputs() inconsistent to the CONFIG_ARCH_ROMGETC version of fputs() above.
##########
File path: libs/libc/stdio/lib_vfprintf.c
##########
@@ -37,23 +37,20 @@ int vfprintf(FAR FILE *stream, FAR const IPTR char *fmt, va_list ap)
struct lib_stdoutstream_s stdoutstream;
int n = ERROR;
- if (stream)
- {
- /* Wrap the stream in a stream object and let lib_vsprintf
- * do the work.
- */
-
- lib_stdoutstream(&stdoutstream, stream);
-
- /* Hold the stream semaphore throughout the lib_vsprintf
- * call so that this thread can get its entire message out
- * before being pre-empted by the next thread.
- */
-
- lib_take_semaphore(stream);
- n = lib_vsprintf(&stdoutstream.public, fmt, ap);
- lib_give_semaphore(stream);
- }
+ /* Wrap the stream in a stream object and let lib_vsprintf
+ * do the work.
+ */
+
+ lib_stdoutstream(&stdoutstream, stream);
+
+ /* Hold the stream semaphore throughout the lib_vsprintf
+ * call so that this thread can get its entire message out
+ * before being pre-empted by the next thread.
+ */
+
+ lib_take_semaphore(stream);
+ n = lib_vsprintf(&stdoutstream.public, fmt, ap);
Review comment:
Separately from this PR, it might be a good thing to replace *sprintf() with *snprintf() in the future, wherever possible.
Since it was already lib_vsprintf() previously, it doesn't have to be done right now.
##########
File path: libs/libc/string/lib_strndup.c
##########
@@ -53,24 +53,22 @@
FAR char *strndup(FAR const char *s, size_t size)
{
FAR char *news = NULL;
- if (s)
Review comment:
Same here...
##########
File path: libs/libc/string/lib_strdup.c
##########
@@ -36,13 +36,11 @@
FAR char *strdup(FAR const char *s)
{
FAR char *news = NULL;
- if (s)
Review comment:
Same here
##########
File path: libs/libc/string/lib_strpbrk.c
##########
@@ -33,15 +33,6 @@
#undef strpbrk /* See mm/README.txt */
FAR char *strpbrk(FAR const char *str, FAR const char *charset)
{
- /* Sanity checking */
-
-#ifdef CONFIG_DEBUG_FEATURES
- if (!str || !charset)
- {
- return NULL;
- }
-#endif
-
Review comment:
Same comment as earlier... do we really want to remove the debug checks?
##########
File path: libs/libc/string/lib_strchr.c
##########
@@ -48,19 +48,16 @@
#undef strchr /* See mm/README.txt */
FAR char *strchr(FAR const char *s, int c)
{
- if (s)
Review comment:
Same comment here as memchr() above: really eliminate the NULL pointer check?
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1045150385
> I recognize that by having the various NULL checks, some compilers may issue warnings about impossible code paths, etc. However I feel it is safer to have the NULL checks. One idea could be to make '-Werror=nonnull-compare' and '-Werror=format-truncation' dependent on (yet another) Kconfig and make the compiler warnings and the NULL pointer checks mutually exclusive. So either you have compiler warnings or NULL checks, but not both or neither. My 2 cents. :-)
Do you propose leave error checks and suppress compiler warnings?
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1045978970
> I recognize that by having the various NULL checks, some compilers may issue warnings about impossible code paths, etc.
Yes, but this is just for some special functions(e.g. strcpy) which is known by compiler internally.
> However I feel it is safer to have the NULL checks. One idea could be to make '-Werror=nonnull-compare' and '-Werror=format-truncation' dependent on (yet another) Kconfig and make the compiler warnings and the NULL pointer checks mutually exclusive. So either you have compiler warnings or NULL checks, but not both or neither. My 2 cents. :-)
Kconfig means user can enable it, and then the compiler throw many warning. Here is my thought:
1. Compiler will throw warning if caller pass NULL pointer to these special function
2. NULL pointer access normally generate the hardware exception by MPU or MMU
3. The affected functions are very small and simple
So, the side effect is minor.
--
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 edited a comment on pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1042750799
Do we have `-Werror=nonnull` enabled?
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810102665
##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -205,11 +205,11 @@ static int work_thread_create(FAR const char *name, int priority,
FAR struct kwork_wqueue_s *wqueue)
{
FAR char *argv[2];
- char args[16];
+ char args[32];
int wndx;
int pid;
- snprintf(args, 16, "0x%" PRIxPTR, (uintptr_t)wqueue);
+ snprintf(args, 32, "0x%" PRIxPTR, (uintptr_t)wqueue);
Review comment:
Done.
##########
File path: libs/libc/string/lib_strdup.c
##########
@@ -36,13 +36,11 @@
FAR char *strdup(FAR const char *s)
{
FAR char *news = NULL;
- if (s)
+
+ news = (FAR char *)lib_malloc(strlen(s) + 1);
Review comment:
Done.
##########
File path: libs/libc/time/lib_asctimer.c
##########
@@ -70,10 +70,13 @@ static const char * const g_mon_name[12] =
FAR char *asctime_r(FAR const struct tm *tp, FAR char *buf)
{
- snprintf(buf, 26, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
+ char tmp[128];
+
+ snprintf(tmp, 128, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
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 pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1048811822
@hartmannathan what you opinion?
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1042983225
> Do we have `-Werror=nonnull` enabled?
We enable -Wall in all Make.defs and enable -Werror int tools/ci/cibuild.sh.
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810108886
##########
File path: libs/libc/stdio/lib_fputs.c
##########
@@ -93,16 +93,6 @@ int fputs(FAR const IPTR char *s, FAR FILE *stream)
{
int nput;
- /* Make sure that a string was provided. */
-
-#ifdef CONFIG_DEBUG_FEATURES /* Most parameter checking is disabled if DEBUG is off */
- if (s == NULL || stream == NULL)
- {
- set_errno(EINVAL);
- return EOF;
- }
-#endif
-
Review comment:
> Why eliminate the optional check?
Because compiler issue the following warning when -fno-builtin is removed from Make.defs:
```
stdio/lib_fputs.c: In function 'fputs':
Error: stdio/lib_fputs.c:99:9: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s == NULL || stream == NULL)
^
Error: stdio/lib_fputs.c:99:27: error: nonnull argument 'stream' compared to NULL [-Werror=nonnull-compare]
if (s == NULL || stream == NULL)
```
>
> Removing it makes this version of fputs() inconsistent to the CONFIG_ARCH_ROMGETC version of fputs() above.
Remove the check in CONFIG_ARCH_ROMGETC version too.
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810111498
##########
File path: libs/libc/string/lib_strchr.c
##########
@@ -48,19 +48,16 @@
#undef strchr /* See mm/README.txt */
FAR char *strchr(FAR const char *s, int c)
{
- if (s)
+ for (; ; s++)
{
- for (; ; s++)
+ if (*s == c)
{
- if (*s == c)
- {
- return (FAR char *)s;
- }
+ return (FAR char *)s;
+ }
- if (!*s)
- {
- break;
- }
+ if (!*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] xiaoxiang781216 commented on a change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r811134387
##########
File path: libs/libc/string/lib_memchr.c
##########
@@ -49,17 +49,14 @@ FAR void *memchr(FAR const void *s, int c, size_t n)
{
FAR const unsigned char *p = (FAR const unsigned char *)s;
- if (s)
+ while (n--)
{
- while (n--)
+ if (*p == (unsigned char)c)
Review comment:
the code may want to avoid depend on whether char extend as unsigned or signed.
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1050024579
ping @hartmannathan
--
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 change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r809137279
##########
File path: libs/libc/time/lib_asctimer.c
##########
@@ -70,10 +70,13 @@ static const char * const g_mon_name[12] =
FAR char *asctime_r(FAR const struct tm *tp, FAR char *buf)
{
- snprintf(buf, 26, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
+ char tmp[128];
+
+ snprintf(tmp, 128, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
Review comment:
```suggestion
snprintf(tmp, sizeof(tmp), "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n",
```
##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -205,11 +205,11 @@ static int work_thread_create(FAR const char *name, int priority,
FAR struct kwork_wqueue_s *wqueue)
{
FAR char *argv[2];
- char args[16];
+ char args[32];
int wndx;
int pid;
- snprintf(args, 16, "0x%" PRIxPTR, (uintptr_t)wqueue);
+ snprintf(args, 32, "0x%" PRIxPTR, (uintptr_t)wqueue);
Review comment:
```suggestion
snprintf(args, sizeof(args), "0x%" PRIxPTR, (uintptr_t)wqueue);
```
##########
File path: libs/libc/string/lib_strchr.c
##########
@@ -48,19 +48,16 @@
#undef strchr /* See mm/README.txt */
FAR char *strchr(FAR const char *s, int c)
{
- if (s)
+ for (; ; s++)
{
- for (; ; s++)
+ if (*s == c)
{
- if (*s == c)
- {
- return (FAR char *)s;
- }
+ return (FAR char *)s;
+ }
- if (!*s)
- {
- break;
- }
+ if (!*s)
Review comment:
```suggestion
if (*s == '\0')
```
##########
File path: libs/libc/string/lib_strdup.c
##########
@@ -36,13 +36,11 @@
FAR char *strdup(FAR const char *s)
{
FAR char *news = NULL;
- if (s)
+
+ news = (FAR char *)lib_malloc(strlen(s) + 1);
Review comment:
```suggestion
FAR char *news = (FAR char *)lib_malloc(strlen(s) + 1);
```
optional
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810265977
##########
File path: libs/libc/stdio/lib_vfprintf.c
##########
@@ -37,23 +37,20 @@ int vfprintf(FAR FILE *stream, FAR const IPTR char *fmt, va_list ap)
struct lib_stdoutstream_s stdoutstream;
int n = ERROR;
- if (stream)
- {
- /* Wrap the stream in a stream object and let lib_vsprintf
- * do the work.
- */
-
- lib_stdoutstream(&stdoutstream, stream);
-
- /* Hold the stream semaphore throughout the lib_vsprintf
- * call so that this thread can get its entire message out
- * before being pre-empted by the next thread.
- */
-
- lib_take_semaphore(stream);
- n = lib_vsprintf(&stdoutstream.public, fmt, ap);
- lib_give_semaphore(stream);
- }
+ /* Wrap the stream in a stream object and let lib_vsprintf
+ * do the work.
+ */
+
+ lib_stdoutstream(&stdoutstream, stream);
+
+ /* Hold the stream semaphore throughout the lib_vsprintf
+ * call so that this thread can get its entire message out
+ * before being pre-empted by the next thread.
+ */
+
+ lib_take_semaphore(stream);
+ n = lib_vsprintf(&stdoutstream.public, fmt, ap);
Review comment:
Ok, let's do in the next 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] xiaoxiang781216 commented on a change in pull request #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#discussion_r810115503
##########
File path: libs/libc/string/lib_memchr.c
##########
@@ -49,17 +49,14 @@ FAR void *memchr(FAR const void *s, int c, size_t n)
{
FAR const unsigned char *p = (FAR const unsigned char *)s;
- if (s)
Review comment:
All change in this patch fix the compiler warning when we remove -fno-builtin from Make.defs
##########
File path: libs/libc/string/lib_strchr.c
##########
@@ -48,19 +48,16 @@
#undef strchr /* See mm/README.txt */
FAR char *strchr(FAR const char *s, int c)
{
- if (s)
Review comment:
To fix the compiler warning
##########
File path: libs/libc/string/lib_strndup.c
##########
@@ -53,24 +53,22 @@
FAR char *strndup(FAR const char *s, size_t size)
{
FAR char *news = NULL;
- if (s)
Review comment:
Fix warning:
```
string/lib_strndup.c: In function 'strndup':
Error: string/lib_strndup.c:56:6: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s)
```
##########
File path: libs/libc/string/lib_strdup.c
##########
@@ -36,13 +36,11 @@
FAR char *strdup(FAR const char *s)
{
FAR char *news = NULL;
- if (s)
Review comment:
Fix the warning:
```
string/lib_strdup.c: In function 'strdup':
Error: string/lib_strdup.c:39:6: error: nonnull argument 's' compared to NULL [-Werror=nonnull-compare]
if (s)
^
```
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519#issuecomment-1042750799
Do we have `-Wnonnull` enabled?
--
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 #5519: Fix -Werror=nonnull-compare and -Werror=format-truncation=
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5519:
URL: https://github.com/apache/incubator-nuttx/pull/5519
--
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