C - strcat and string/char functions, return variables

marquisor · 10-19-2010, 09:01 PM

What'swrong with this little code?

Code:

/*
 * aufg36.c
 *
 *  Created on: 20.10.2010
 *      Author: marquisor
 */

												/* Includes */

#include <stdio.h>
#include <string.h>

												/* Funktionsprototypen */

char * terminate_symbols(char [80]);

												/* Funktionen */

char * terminate_symbols(char text[80]){
	int i,length;
	char newtext[80];
	length=strlen(text);
	for (i=0; i<=length; i++){
		if (text[i] == '.' || text[i] == ',' || text[i] == '!' || text[i] == ':' || text[i] == ';' || text[i] == '?' || text[i] == ' ')
			continue;
		strcat(newtext,(char) text[i]);
	}
	return newtext;
}

												/* Hauptprogramm */

int main(){
	char input[80];
	char new_string[80];

	printf("\n\tSatz-Terminator 1\n\n");
	printf("\nBitte einen beliebigen Satz eingeben. Maximale Länge 80 Zeichen!\n\n");
	gets(input);

	strcpy(new_string,terminate_symbols(input));

	printf("\nDer neue Text lautet: \n");
	printf(" %s",new_string);

	return 0;
}

Purpose: remove all symbols from text and print it out.

I don't know, something seems to be wrong with the "terminate_symbols" function, but i can't get it.

Thx in advance.
Regards

marquisor

Dark_Helmet · 10-19-2010, 11:02 PM

First, you never initialize the variable newtext.

Second, you're misusing strcat. From the strcat man page:

Code:

SYNOPSIS
       #include <string.h>

       char *strcat(char *dest, const char *src);

The second parameter (src) is a const char * -- not a char. You can't change how strcat works by typecasting one of its arguments.

EDIT: And incidentally, your typecast doesn't do anything because text[i] is already a char. I'm guessing you got a warning from the compiler and you thought that an explicit typecast might make the warning go away,but you didn't take the time to figure out what the warning was trying to tell you.

neonsignal · 10-19-2010, 11:07 PM

There are several issues.

1. The string pointer newtext being returned by terminate_symbols has been declared as a local variable in the function. This means that the string is no longer in scope (typically it might be created on the stack, so the old space might be overwritten by subsequent calls, including the call to the strcpy). You may find that it works in your example, but the behaviour is not defined, so it is a recipe for disaster.

You can fix this by passing in a pointer to new_string (which will also save having an extra strcpy after the function returns).

2. The newtext buffer was not initialized, so calling strcat will have undefined behaviour (since the end of the string is defined by a zero byte). Again, it might work through luck, but don't rely on it.

3. The function strcat copies the entire string that it is given, not just a single character. It requires a pointer to a string, not a single character (your cast is covering this up, and is the reason for the segmentation fault).

You would probably be better off copying the character explicitly, rather than using a library function (assuming this is a teaching exercise).

4. Functions such as gets and strcat are inherently unsafe, because they have no limit placed on how many characters they copy. This leads to buffer overflows (since C does not have automatic array index checking). It is worth learning alternatives such as fgets and strncat early on before you get bitten by them!