diff mbox series

[v2] time: Remove alloca() from getdate

Message ID 20230426152244.1981882-1-josimmon@redhat.com
State Superseded
Headers
Series [v2] time: Remove alloca() from getdate |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Joe Simmons-Talbott April 26, 2023, 3:22 p.m. UTC 
  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(-)

Comments

Siddhesh Poyarekar May 2, 2023, 4:38 p.m. UTC | #1 
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;
diff mbox series

Patch

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;