[prog] [C] strings bug

[Conor Daly]

On Wed, Apr 09, 2003 at 05:22:14PM -0700 or thereabouts, Laurel Fan wrote:
> 
> See explanation inline with the quoted code:
> 
> On Wed, Apr 09, 2003 at 10:55:02PM +0100, Conor Daly wrote:
> > char *strset(char *dest, const char *src) {
> > 
> > 	/* create a tmp string to hold 'dest' while we free that */
> > 	char *tmp;
> 
> The problem is with the inital value of tmp.  When you declare a local
> variable, its initial value is undefined.  See below...
> 
> > 	if(dest)
> > 	{
> > 
> > 	/* take a copy of dest */
> > 	if( (tmp = strdup(dest)) == NULL)
> 
> tmp might be set here, but only if dest was not NULL.
> 
> > 		{
> > 			return(dest); /* we failed to make a copy so bail
> > 					out */
> > 		}
> > 	/* then free it */
> > 	free(dest);
> > 	}
> > 	/* now use strdup() to replace the old string with the new */
> > 	if( (dest = strdup(src)) == NULL) {
> > 		  printf("Failed to set dest\n");
> > 	/* the copy failed so we want to return the original string
> > 	unchanged.
> > 	 */
> > 		return(tmp);
> > 	} else {
> > 		/* free the tmp string */
> > 		free(tmp);
> 
> Here, it is possible that the value of tmp is not defined (which means
> it could be anything), so freeing this value can have unpredictable
> results.

Ah, I see...
tmp is undefined if 'dest = strdup()' fails and it wasn't set earlier.
 
> > 		/* and return the new pointer to dest */
> > 		return(dest);
> > 	}
> > }
> > /* *************end strset()**************************/
> 
> That said, the way I would do it avoids copying dest altogether:
> 
> char *strset(char *string_to_replace, onst char *src) {
>     /*
>         Return a newly allocated copy of src.
>         If successful, string_to_replace will be freed.
>         If unsuccessful, no new memory will be allocated, and
> 	string_to_replace will be returned unchanged.
>     */
> 
>     char *new_copy_of_src = strdup(src);
> 
>     if (new_copy_of_src == NULL) {
>         /* printf("Copying src string failed\n"); */
>         return(string_to_replace);
>     } else {
> 	/* printf("Copying src string succeeded\n") */
>         free(string_to_replace);
>         return(new_copy_of_src);
>     }
> }

Now, this is _so_ nice.  Why can't I think in 'elegant' like that!?

Thanks heaps,
Conor
-- 
Conor Daly 
Met Eireann, Glasnevin Hill, Dublin 9, Ireland
Ph +353 1 8064276 Fax +353 1 8064247
------------------------------------
bofh.irmet.ie running RedHat Linux  9:07am  up 6 days, 22:58, 10 users,  load average: 0.20, 0.12, 0.04


**********************************************************************
This e-mail and any files transmitted with it are confidential 
and intended solely for the addressee. If you have received
this email in error please notify the sender.
This e-mail message has also been scanned for the
presence of computer viruses.
**********************************************************************