replace alloca with strdup

replace alloca with strdup

[Han Boetes]

Hi,

While looking at some compilerwarnings I found an alloca and an
strcpy.  And I realized this can also be done as follows:


Index: xterm.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xterm.c,v
retrieving revision 1.864
diff -u -p -r1.864 xterm.c
--- xterm.c	10 May 2005 09:19:19 -0000	1.864
+++ xterm.c	21 May 2005 16:23:31 -0000
@@ -7635,8 +7635,9 @@ x_connection_closed (dpy, error_message)
   Lisp_Object frame, tail;
   int count;
 
-  error_msg = (char *) alloca (strlen (error_message) + 1);
-  strcpy (error_msg, error_message);
+  if ((error_msg = strdup(error_message)) == NULL)
+    errx(1, "Out of memory.");
+
   handling_signal = 0;
 
   /* Prevent being called recursively because of an error condition


Advantages are:

  - simpler code
  - strdup is more portable
  - error handling
  - you don't have to worry about strdup concerning security

I hope you like the idea.



# Han

Re: replace alloca with strdup

[Masatake YAMATO]

Hi,

> -  error_msg = (char *) alloca (strlen (error_message) + 1);
> -  strcpy (error_msg, error_message);
> +  if ((error_msg = strdup(error_message)) == NULL)
> +    errx(1, "Out of memory.");
> +

I wonder who will release the memory chunk allocated by strdup.

Re: replace alloca with strdup

[David Kastrup]

Han Boetes <han@mijncomputer.nl> writes:

> While looking at some compilerwarnings I found an alloca and an
> strcpy.  And I realized this can also be done as follows:
>
>
> Index: xterm.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/xterm.c,v
> retrieving revision 1.864
> diff -u -p -r1.864 xterm.c
> --- xterm.c	10 May 2005 09:19:19 -0000	1.864
> +++ xterm.c	21 May 2005 16:23:31 -0000
> @@ -7635,8 +7635,9 @@ x_connection_closed (dpy, error_message)
>    Lisp_Object frame, tail;
>    int count;
>  
> -  error_msg = (char *) alloca (strlen (error_message) + 1);
> -  strcpy (error_msg, error_message);
> +  if ((error_msg = strdup(error_message)) == NULL)
> +    errx(1, "Out of memory.");
> +
>    handling_signal = 0;
>  
>    /* Prevent being called recursively because of an error condition
>
>
> Advantages are:
>
>   - simpler code
>   - strdup is more portable
>   - error handling
>   - you don't have to worry about strdup concerning security
>
> I hope you like the idea.

Disadvantage: memory leak.

I don't like it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: replace alloca with strdup

[Andreas Schwab]

Han Boetes <han@mijncomputer.nl> writes:

> Advantages are:
>
>   - simpler code

Not really.

>   - strdup is more portable

Not more than alloca.

>   - error handling

Which error?

>   - you don't have to worry about strdup concerning security

The code you changed is already obviously safe.

> I hope you like the idea.

No.  Nobody is reclaiming the storage, so you introduced a memory leak.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: replace alloca with strdup

[Richard Stallman]

    While looking at some compilerwarnings I found an alloca and an
    strcpy.

What is wrong with that?  I don't see any problem in it.

	     And I realized this can also be done as follows:

    +  if ((error_msg = strdup(error_message)) == NULL)
    +    errx(1, "Out of memory.");

This creates a memory leak.