[alt.sources.wanted] Please stop the argument on "strdup

cheewai@spinifex.eecs.unsw.oz (Wai Yeung) (02/23/91)

In my opinion, I think all the hassles about the code for "strdup()" should
stop and let everyone gets back to writig better programs. To settle all
arguments, I now enclose the few initial news items that involved the
questionable code:

-------------------------------------------

From: jeff@onion.rain.com (Jeff Beadles)
Newsgroups: comp.sources.wanted,alt.sources.wanted
Subject: Re: What is strdup() supposed to do? Does anyone have a copy?
Message-ID: <1991Feb17.164731.7564@onion.rain.com>
Date: 17 Feb 91 16:47:31 GMT
References: <1991Feb14.050716.9501@shibaya.lonestar.org> <1991Feb17.045913.17126@sbcs.sunysb.edu>
Lines: 62

In <1991Feb17.045913.17126@sbcs.sunysb.edu> cbrown@eeserv1.ic.sunysb.edu
 (Charles T Brown) writes:
>
>Well, having learned C with Microsoft C, I looked it up.
>
>Basically:
>
>char * strdup (s)
>
>char * s;
>{
>	char * rets;
>
>	rets=malloc (sizeof (char) * strlen (s));
>
>	strcpy (rets, s);
>
>	return rets;
>}

Please folks, if you're going to answer a question, please do it correctly. 
There are two rather serious bugs with this function.

1)  If malloc returns an error, it's ignored and the program blindly continues.

2)  You're not malloc'ing enough space for the string!  You **MUST** malloc
strlen(str) + 1!  The +1 is for the trailing null that is added to the end
of every strcpy'ed string.

Actually, malloc and strlen should also be declared, but that's possibly
different for various systems.

Strcpy could also return an error, but I don't think that I've ever seen
someone check for it. :-)

Here's an updated and working version of strdup()

char *
strdup (s)
char *s;
{
	static char *rets;
	extern char *malloc();	/* Might be extern void *malloc(); */
	extern int strlen();

	rets=malloc (sizeof (char) * strlen (s) + sizeof(char) );

	/* Malloc failed.  Oops! */
	if (!rets) {
		return((char *)0);
	}

	(void) strcpy (rets, s);

	return (rets);
}

	-Jeff
-- 
Jeff Beadles		jeff@onion.rain.com

....... The above was the first reply from Jeff. .........

From: dag@persoft.com (Daniel A. Glasser)
Newsgroups: comp.sources.wanted,alt.sources.wanted
Subject: Re: What is strdup() supposed to do? Does anyone have a copy?
Message-ID: <1991Feb18.154159.430@persoft.com>
Date: 18 Feb 91 15:41:59 GMT
References: <1991Feb14.050716.9501@shibaya.lonestar.org> <1991Feb17.045913.17126@sbcs.sunysb.edu> <1991Feb17.164731.7564@onion.rain.com>
Organization: Persoft, Inc.
Lines: 71

In article <1991Feb17.164731.7564@onion.rain.com> jeff@onion.rain.com (Jeff Beadles) writes:

....... Code extracted from Jeff's reply deleted ..........

Two minor nits...

Nit #1:	Why is rets static?  Is there any reason to not leave it as automatic?
        The value is not used between calls, and why waste data space when the
	stack is available for these purposes?  I'd suggest leaving the "static"
	off.

Nit #2:	You should simplify your expression in the "malloc()", that is,
		sizeof(char) * strlen(foo) + sizeof(char)
	could be written
		sizeof(char) * (strlen(foo) + 1)
	I realize that this doesn't make it any less work for the CPU
	(both are 1 add and 1 multiply), but to the latter form makes
	it clearer what is being done (allocating enough space for
	strlen(foo)+1 chars).

Also note that some compilers will complain about the extern function
declarations within function scope.  Declarations of these functions should
be left to header files (if they are declared in header files) so as not to
get in trouble with ANSI-C compliant implementations.

A shorter version (assuming declarations of malloc(), strlen(), and strcpy(),
and a definition for NULL in some header file or preceeding this function
in the source file is:

char *strdup(orig)	/* Allocate a new copy of a string 	*/
char *orig;		/* a pointer to the original string 	*/
{
	char *copy;	/* where we keep the copy.		*/

	if (orig == (char *)NULL)	/* If the original is NULL	*/
		return (char *)NULL;	/* so is the result.		*/

	if (copy = malloc((strlen(orig) + 1) * sizeof(char)))
		strcpy(copy, orig);	/* if malloc() worked, copy the	*/
					/* string data.			*/
	return copy;			/* return the result of malloc() */
} /* end of strdup() */

I did add one more error check, and I used some shorthand (the assignment
within the if()) that some people may take exception to, however this is
just about as efficient (code wise) as you can get and still insure
robust behavior of the function.  If you drop the NULL check on the
original, you can have it even smaller but at the risk that the strcpy
or strlen call will die a horrible death.
-- 
          Daniel A. Glasser  |  Persoft, Inc.  |  dag@persoft.com
                "Their brains were small, and they died."

.......... I think this is where the argument started ........

From: jeff@onion.rain.com (Jeff Beadles)
Newsgroups: comp.sources.wanted,alt.sources.wanted
Subject: Re: What is strdup() supposed to do? Does anyone have a copy?
Message-ID: <1991Feb19.004526.16033@onion.rain.com>
Date: 19 Feb 91 00:45:26 GMT
References: <1991Feb17.045913.17126@sbcs.sunysb.edu> <1991Feb17.164731.7564@onion.rain.com> <1991Feb18.154159.430@persoft.com>

 In <1991Feb18.154159.430@persoft.com> dag@persoft.com (Daniel Glasser) writes:
 >In article <1991Feb17.164731.7564@onion.rain.com> I wrote:
 >>Here's an updated and working version of strdup()
 >>
 >>char *
 >>strdup (s)
 >>char *s;
 >>{
 >>	static char *rets;
 >>	extern char *malloc();	/* Might be extern void *malloc(); */
 >>	extern int strlen();
 >>
 >>	rets=malloc (sizeof (char) * strlen (s) + sizeof(char) );
 >>
 >>	/* Malloc failed.  Oops! */
 >>	if (!rets) {
 >>		return((char *)0);
 >>	}
 >>
 >>	(void) strcpy (rets, s);
 >>
 >>	return (rets);
 >>}
 >
 >Two minor nits...
 >
.... Argument about the first point mentioned by Daniel deleted .....
 
 >Nit #2:	You should simplify your expression in the "malloc()", that is,
 >		sizeof(char) * strlen(foo) + sizeof(char)
 >	could be written
 >		sizeof(char) * (strlen(foo) + 1)
 
 >	I realize that this doesn't make it any less work for the CPU
 >	(both are 1 add and 1 multiply), but to the latter form makes
 >	it clearer what is being done (allocating enough space for
 >	strlen(foo)+1 chars).
 
Untrue.  What if sizeof(char) != 1?  Yours will break, and mine will work.
 
{note:  I don't personally know of places where this isn't true, but then
        again, some people think that sizeof (int) == sizeof (char *) :-}

Also, sizeof() isn't taking anything extra.  It's changed to a '1' by the
COMPILER.  Overall, I stand by my choice of + sizeof(char).

I wrote this as an example for folks needing the functionality, not as a
finely-tuned piece of code.  You're more than welcome to improve on it.

	-Jeff
-- 
Jeff Beadles		jeff@onion.rain.com

----------------------------------------

I don't intend to offend anyone. But I think a lot of us who are reading news
will find it a bit tough just to go through 30 new news items a day for this
group, with 1/3 of them arguing about just a few lines of code. I hope this
can settle the argument once and forever!

PS: Sorry about the length of the item! Also, if this article did offend anyone,please accept my most humble apology!