[prog] [C] strings bug

Conor Daly conor.daly at oceanfree.net
Wed Apr 9 22:55:02 EST 2003


/* Hi all,

I've put together a function or two to do basic string handling in C.  I
have the functions strset() and stradd() to take the place of strcpy() and
strcat() using malloc to guard against buffer overruns.  Unfortunately,
I'm not doing it right and am getting _interesting_ results from my
strset() function:

I've put comment delimiters about the place so that, if you save this
email and add a C comment at the top of the file, it should compile with:

gcc -lm -o strset <this-filename>

to produce the effect.  The function is starting to produce some strange
effects in some of my programs and is starting to upset me.  Any pointers
(dereferenced or otherwise :-) will be gratefully received...

Thanks,

Conor

/* This is main.c */
#include<stdio.h>
#include<string.h>
#include<stdarg.h>

int main(void);
int printout(void);
char *strdup2(const char *, const char *);
char *strset(char *, const char *);
char *stradd(char *, const char *);

	char *string1;
	char *string2;
	char *string3;

int main(void)
{

	printout();

	string1 = strset(string1, "Test");/**/
	string3 = strset(string3, "Addition");
	printout();

	string2 = strset(string2, "Test");/**/
	string1 = strset(string1, "Testing");/**/
	printout();

	string1 = stradd(string1, ", Testing...");/**/
	string3 = strdup2(string3, "Addition");
	printout();

	printf("%s, %s\n",  strset(string3, "Direct print"),  strset(string3, " on screen"));
}

int printout(void)
{
	printf("\n");
	printf("1: %s\n", string1);
	printf("2: %s\n", string2);
	printf("3: %s\n", string3);
	printf("\n");

	return(0);
}
/* End of main.c */

/*
# result of run...
1: (null)
2: (null)
3: (null)


1: Test
2: (null)
3: Addition


1: Testing
2: Test
3: p±@p±@


1: Testing, Testing...
2: Test
3: Addition

Direct print, ±@±@
# end of run

If I replace the call to strset() with a call to strdup(), all is well:

# result of run
1: (null)
2: (null)
3: (null)


1: Test
2: (null)
3: Addition


1: Testing
2: Test
3: Addition


1: Testing, Testing...
2: Test
3: AdditionAddition

Direct print,  on screen
# end of run

Unfortunately, a call to strdup() by itself is a memory leak since it
malloc()s a new string and returns a pointer to the new string.  If the
destination string is not free()ed first, it gets dereferenced but not
free()ed and leaks memory.  The idea of strset() is to remove the memory
management _out_ of the main function so that an strcat() style call can
be made.

strset() looks like this:

/* *************strset()**************************
 * Uses strdup() to set an existing string
 */
char *strset(char *dest, const char *src) {
	/* This is just a wrapper for strdup() which handles
	 * all the necessary stuff
	 * Since strdup() allocates a new string, we need to
	 * set the target to become the returned pointer
	 *
	 * Useage: dest = strset(dest, src);
	 *
	 * Unfortunately, we need the address of the old string so we 
	 * can free that, otherwise we get a memory leak.
	 *
	 */


	/* create a tmp string to hold 'dest' while we free that */
	char *tmp;
	
	return(strdup(src));/* This is a memory leak */
	/* comment out the line above to see the bug in action */

	printf("in function strset()\n");
	if(dest)
	{

	/* take a copy of dest */
	if( (tmp = strdup(dest)) == 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);
		/* and return the new pointer to dest */
		return(dest);
	}
}
/* *************end strset()**************************/
/*
stradd() is:

/* *************stradd()**************************
 * Uses strdup2() to concatenate two existing 
 * strings
 */
char *stradd(char *dest, const char *src) {
	
	/* This is just a wrapper for strdup2() which handles 
	 * all the necessary stuff
	 * Since strdup2() allocates a new string, we need to 
	 * set the target to become the returned pointer.
	 *
	 * Useage: dest = stradd(dest, src);
	 *
	 */
	
	/* create a tmp string to hold 'dest' while we free that */
	char *tmp=NULL;
	
	if(dest) { /* If dest exists */
		/* take a copy of dest */
		if( (tmp = strdup(dest)) == NULL) return(dest);
		/* we failed to make a copy so bail out */
		/* then free it */
		free(dest);
	}
	/* now use strdup2() to concat the two strings */
	if( (dest = strdup2(tmp, src)) == NULL) {
		/* the copy failed so we want to return the original
			string unchanged.
		 */
		return(tmp);
	} else {
		/* free the tmp string */
		free(tmp);
		/* and return the new pointer to s1 */
		return(dest);
	}
}
/* *************end stradd()**************************/
/*
strdup2() looks like:

/* ************strdup2()*************************
 * This extends the standard strdup() function to
 * to do for strcat() what 
 * strdup() does for strcpy()
 */
char *strdup2(const char *s1, const char *s2)
{
   char *s;
   int strsize=1;

	strsize += strlen(s1);
	strsize += strlen(s2);
	
	if ((s=(char *)malloc(strsize + 1)) == NULL) {
		return NULL;
	}

	/* the following initialises the string.  s[0]=0;s[strsize]=0;
	 * would work too.
	 */
	memset(s, 0, strsize + 1);
	/* from here on in we don't let str* routines near s[strsize]
	 * so we know it will stay 0.
	 */
	strncpy(s, s1, strsize);
	strncat(s, s2, strsize - strlen(s));

	return s;
}
/* ************end strdup2()*************************/
/*


-- 
Conor Daly <conor.daly at oceanfree.net>

Domestic Sysadmin :-)
---------------------
Faenor.cod.ie
 10:22pm  up 20 days,  3:58,  0 users,  load average: 0.00, 0.00, 0.00
Hobbiton.cod.ie
 10:22pm  up 20 days,  3:57,  1 user,  load average: 0.00, 0.00, 0.00 */


More information about the Programming mailing list