[PATCH v2] time: Remove alloca() from getdate

[PATCH v2] time: Remove alloca() from getdate

[Joe Simmons-Talbott]

Changes to v1:
  * Add error handling for __strndup()

--8<--
Reduce the usage of alloca() to the bare minimum to avoid the potential
for stack overflow.  Use __strndup to simplify the code.
---
 time/getdate.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/time/getdate.c b/time/getdate.c
index c5c8378707..2f0ebbe097 100644
--- a/time/getdate.c
+++ b/time/getdate.c
@@ -26,7 +26,6 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include <ctype.h>
-#include <alloca.h>
 
 #define TM_YEAR_BASE 1900
 
@@ -153,26 +152,14 @@ __getdate_r (const char *string, struct tm *tp)
 
   if (inlen < oldlen)
     {
-      bool using_malloc = false;
-
-      if (__libc_use_alloca (inlen + 1))
-	instr = alloca (inlen + 1);
-      else
+      instr = __strndup(string, inlen);
+      if (instr == NULL)
 	{
-	  instr = malloc (inlen + 1);
-	  if (instr == NULL)
-	    {
-	      fclose (fp);
-	      return 6;
-	    }
-	  using_malloc = true;
+	  fclose(fp);
+	  return 6;
 	}
-      memcpy (instr, string, inlen);
-      instr[inlen] = '\0';
+	
       string = instr;
-
-      if (!using_malloc)
-	instr = NULL;
     }
 
   line = NULL;
-- 
2.39.2

Re: [PATCH v2] time: Remove alloca() from getdate

[Siddhesh Poyarekar]

On 2023-04-26 11:22, Joe Simmons-Talbott via Libc-alpha wrote:
> Changes to v1:
>    * Add error handling for __strndup()
> 
> --8<--
> Reduce the usage of alloca() to the bare minimum to avoid the potential
> for stack overflow.  Use __strndup to simplify the code.
> ---
>   time/getdate.c | 23 +++++------------------
>   1 file changed, 5 insertions(+), 18 deletions(-)

+1, I don't see a good reason to prematurely optimize this path given 
that file operations on $DATEMSK should dominate any profile in this 
function.

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> diff --git a/time/getdate.c b/time/getdate.c
> index c5c8378707..2f0ebbe097 100644
> --- a/time/getdate.c
> +++ b/time/getdate.c
> @@ -26,7 +26,6 @@
>   #include <unistd.h>
>   #include <sys/stat.h>
>   #include <ctype.h>
> -#include <alloca.h>
>   
>   #define TM_YEAR_BASE 1900
>   
> @@ -153,26 +152,14 @@ __getdate_r (const char *string, struct tm *tp)
>   
>     if (inlen < oldlen)
>       {
> -      bool using_malloc = false;
> -
> -      if (__libc_use_alloca (inlen + 1))
> -	instr = alloca (inlen + 1);
> -      else
> +      instr = __strndup(string, inlen);
> +      if (instr == NULL)
>   	{
> -	  instr = malloc (inlen + 1);
> -	  if (instr == NULL)
> -	    {
> -	      fclose (fp);
> -	      return 6;
> -	    }
> -	  using_malloc = true;
> +	  fclose(fp);
> +	  return 6;
>   	}
> -      memcpy (instr, string, inlen);
> -      instr[inlen] = '\0';
> +	
>         string = instr;
> -
> -      if (!using_malloc)
> -	instr = NULL;
>       }
>   
>     line = NULL;