xiaoxiang781216 edited a comment on pull request #5476:
URL: https://github.com/apache/incubator-nuttx/pull/5476#issuecomment-1039106422
> I see 4 issues with this PR.
>
> 1. It changes code not described by the PR (minor zalloc)
because memset(since it become built-in, compiler do more check) report the
wrong usage:
```
memset(ap_list_buffer, 0x0, sizeof(ap_list_buffer));
```
should change to:
```
memset(ap_list_buffer, 0x0, sizeof(*ap_list_buffer));
```
The more simple fix is kmm_zalloc and remove memset.
> 2. It removing all the null checks in the library
Yes, because compiler give the follow warning:
```
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)
^
```
compiler has the internal knowledge about these built-in API.
> 3. increasing stack requirements by 128 in a lib funtion
This is another warning reported by compiler since printf become built-in:
```
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);
~~~~~~~~~~~~~~~~~~~
```
> 4. switching out legacy proven code with know characteristic without being
able to control it seem wrong and coupling it with removing null checks makes
it worse.
Reply before. All these change because GCC give more warning with the
knowledge or assumption about the built-in functions.
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
