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/05/10 12:26:27 UTC

[GitHub] [incubator-nuttx] juniskane opened a new pull request, #6229: libc/string: simplify strrchr

juniskane opened a new pull request, #6229:
URL: https://github.com/apache/incubator-nuttx/pull/6229

   ## Summary
   Do not call strlen() inside strrchr(). Old implementation iterated over string twice, if searched for position was at the beginning. This commit changes strrchr() to scan string only once, regardless of input.
   
   At least my compiler with -O2 was not smart enough to get rid of strlen() call.
   
   strrchr(s, 0) still works correctly with new implementation.
   
   ## Impact
   Minor optimization / code simplification
   
   ## 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.

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] juniskane commented on a diff in pull request #6229: libc/string: simplify strrchr

Posted by GitBox <gi...@apache.org>.
juniskane commented on code in PR #6229:
URL: https://github.com/apache/incubator-nuttx/pull/6229#discussion_r869267336


##########
libs/libc/string/lib_strrchr.c:
##########
@@ -37,15 +37,16 @@
 #undef strrchr /* See mm/README.txt */
 FAR char *strrchr(FAR const char *s, int c)
 {
-  FAR const char *p = &s[strlen(s)];
+  FAR const char *r = NULL;
 
-  for (; p >= s; p--)
+  do
     {
-      if (*p == c)
+      if (*s == (char)c)
         {
-          return (FAR char *)p;
+          r = s;
         }
     }
+  while (*s++);

Review Comment:
   done



##########
libs/libc/string/lib_strrchr.c:
##########
@@ -37,15 +37,16 @@
 #undef strrchr /* See mm/README.txt */
 FAR char *strrchr(FAR const char *s, int c)
 {
-  FAR const char *p = &s[strlen(s)];
+  FAR const char *r = NULL;
 
-  for (; p >= s; p--)
+  do
     {
-      if (*p == c)
+      if (*s == (char)c)

Review Comment:
   removed cast



-- 
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 #6229: libc/string: simplify strrchr

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6229:
URL: https://github.com/apache/incubator-nuttx/pull/6229


-- 
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 diff in pull request #6229: libc/string: simplify strrchr

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6229:
URL: https://github.com/apache/incubator-nuttx/pull/6229#discussion_r869177221


##########
libs/libc/string/lib_strrchr.c:
##########
@@ -37,15 +37,16 @@
 #undef strrchr /* See mm/README.txt */
 FAR char *strrchr(FAR const char *s, int c)
 {
-  FAR const char *p = &s[strlen(s)];
+  FAR const char *r = NULL;
 
-  for (; p >= s; p--)
+  do
     {
-      if (*p == c)
+      if (*s == (char)c)

Review Comment:
   I think the cast is redundant



##########
libs/libc/string/lib_strrchr.c:
##########
@@ -37,15 +37,16 @@
 #undef strrchr /* See mm/README.txt */
 FAR char *strrchr(FAR const char *s, int c)
 {
-  FAR const char *p = &s[strlen(s)];
+  FAR const char *r = NULL;
 
-  for (; p >= s; p--)
+  do
     {
-      if (*p == c)
+      if (*s == (char)c)
         {
-          return (FAR char *)p;
+          r = s;
         }
     }
+  while (*s++);

Review Comment:
   ```suggestion
     while (*s++ != '\0');
   ```



-- 
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 diff in pull request #6229: libc/string: simplify strrchr

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6229:
URL: https://github.com/apache/incubator-nuttx/pull/6229#discussion_r869179992


##########
libs/libc/string/lib_strrchr.c:
##########
@@ -37,15 +37,16 @@
 #undef strrchr /* See mm/README.txt */
 FAR char *strrchr(FAR const char *s, int c)
 {
-  FAR const char *p = &s[strlen(s)];
+  FAR const char *r = NULL;
 
-  for (; p >= s; p--)
+  do
     {
-      if (*p == c)
+      if (*s == (char)c)

Review Comment:
   don't need (char)



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