Re: [PATCH 2/7] stdlib: Simplify getenv

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 15/02/23 14:50, Wilco Dijkstra wrote:
> Hi Adhemerval,
> 
> +  size_t len = __strchrnul (name, '=') - name;
> 
> This is a change in behaviour, ignoring anything behind a '=' in name. What is
> wrong with strlen?

Indeed it is better to keep current semantic.

> 
> +  for (char **ep = __environ; *ep != NULL; ++ep)
>      {
> -      size_t len = strlen (name);
> -#if _STRING_ARCH_unaligned
> -      name_start = *(const uint16_t *) name;
> -#else
> -      name_start = (((const unsigned char *) name)[0]
> -                   | (((const unsigned char *) name)[1] << 8));
> -#endif
> -      len -= 2;
> -      name += 2;
> -
> -      for (ep = __environ; *ep != NULL; ++ep)
> -       {
> -#if _STRING_ARCH_unaligned
> -         uint16_t ep_start = *(uint16_t *) *ep;
> -#else
> -         uint16_t ep_start = (((unsigned char *) *ep)[0]
> -                              | (((unsigned char *) *ep)[1] << 8));
> -#endif
> -
> -         if (name_start == ep_start && !strncmp (*ep + 2, name, len)
> -             && (*ep)[len + 2] == '=')
> -           return &(*ep)[len + 3];
> -       }
> +      if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
> +       return *ep + len + 1;
> 
> It's still useful to keep a check for the first character, eg. name[0] == (*ep)[0] as this
> is about twice as fast as always calling strncmp.

Alright, updated patch below.

From ddfb6419c5e74b34fb22454581fa0636275ec942 Mon Sep 17 00:00:00 2001
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Thu, 9 Feb 2023 10:36:57 -0300
Subject: [PATCH 2/7] stdlib: Simplify getenv

And remove _STRING_ARCH_unaligned usage.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 stdlib/getenv.c | 68 +++++++------------------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

diff --git a/stdlib/getenv.c b/stdlib/getenv.c
index e3157ce2f3..61a42d941d 100644
--- a/stdlib/getenv.c
+++ b/stdlib/getenv.c
@@ -15,76 +15,26 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <endian.h>
-#include <errno.h>
-#include <stdint.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-
-/* Return the value of the environment variable NAME.  This implementation
-   is tuned a bit in that it assumes no environment variable has an empty
-   name which of course should always be true.  We have a special case for
-   one character names so that for the general case we can assume at least
-   two characters which we can access.  By doing this we can avoid using the
-   `strncmp' most of the time.  */
 char *
 getenv (const char *name)
 {
-  char **ep;
-  uint16_t name_start;
-
   if (__environ == NULL || name[0] == '\0')
     return NULL;
 
-  if (name[1] == '\0')
-    {
-      /* The name of the variable consists of only one character.  Therefore
-	 the first two characters of the environment entry are this character
-	 and a '=' character.  */
-#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
-      name_start = ('=' << 8) | *(const unsigned char *) name;
-#else
-      name_start = '=' | ((*(const unsigned char *) name) << 8);
-#endif
-      for (ep = __environ; *ep != NULL; ++ep)
-	{
-#if _STRING_ARCH_unaligned
-	  uint16_t ep_start = *(uint16_t *) *ep;
-#else
-	  uint16_t ep_start = (((unsigned char *) *ep)[0]
-			       | (((unsigned char *) *ep)[1] << 8));
-#endif
-	  if (name_start == ep_start)
-	    return &(*ep)[2];
-	}
-    }
-  else
-    {
-      size_t len = strlen (name);
-#if _STRING_ARCH_unaligned
-      name_start = *(const uint16_t *) name;
-#else
-      name_start = (((const unsigned char *) name)[0]
-		    | (((const unsigned char *) name)[1] << 8));
-#endif
-      len -= 2;
-      name += 2;
-
-      for (ep = __environ; *ep != NULL; ++ep)
-	{
-#if _STRING_ARCH_unaligned
-	  uint16_t ep_start = *(uint16_t *) *ep;
-#else
-	  uint16_t ep_start = (((unsigned char *) *ep)[0]
-			       | (((unsigned char *) *ep)[1] << 8));
-#endif
+  bool single_char = name[1] == '\0';
 
-	  if (name_start == ep_start && !strncmp (*ep + 2, name, len)
-	      && (*ep)[len + 2] == '=')
-	    return &(*ep)[len + 3];
-	}
+  size_t len = strlen (name);;
+  for (char **ep = __environ; *ep != NULL; ++ep)
+    {
+      if (single_char && (*ep)[0] == name[0] && (*ep)[1] == '=')
+	return *ep + 2;
+      else if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+	return *ep + len + 1;
     }
 
   return NULL;
-- 
2.34.1