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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
