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!