Can someone point out the problem in this code?

[Reigning Glory]

#define SIZE1 40
#define SIZE2 1024
#include <stdio.h>
#include <ctype.h>

void Instructions(void);
void generateRandStr(char *);
char userString(char *);
void strFilter(char *, char *, char);

void main(void) {
	char s1 [SIZE1], s2 [SIZE2], c, proceed;// buffer[81];
	
	generateRandStr(s1);
		
	do {
		Instructions();
		puts("\nRandomly Generated String: ");
		puts(s1);
		
		c = userString(s2);
		puts("\nUser Entered String:");
		puts(s2);
		strFilter(s1, s2, c);
	
		puts("\nDo you wish to continue?");
		printf("Press enter to proceed, any other key to exit: ");
		scanf("%*c%c", &proceed);
		if(proceed != '\n')
			puts("\nGoodbye!");
	}	while(proceed == '\n');
}

void Instructions() {
	puts("\nThis program creates a random string of 40 characters.");
	puts("The user enters a string (following certain rules)");
	puts("and then enters a replacement character.");
	puts("The program then compares the user string with the random string");
	puts("and replaces any instances of the same character(s) with");
	puts("the replacement character, and displays the 'filtered' string.");
}

void generateRandStr(char *ps1) {
	int n;
	char *pps1;

	for(pps1 = ps1; pps1 < ps1 + SIZE1; pps1++) {
		n = rand();
		*pps1 = (n % 26) + 'A';
	}	*pps1 = '\0';	
}

char userString(char *ps2) {
	int count, error;
	char c, ch, *pps2;

	do {
		pps2 = ps2;
		count = error = 0;
		puts("\nEnter a string of maximum 20 characters(no #s, symbols, punctuations etc):\n");
		ch = toupper(getchar());
		while(count < SIZE2) {
			while(ch != '\n' && !error) {
				if(ch >= 'A' && ch <= 'Z') {
					*pps2 = ch;
					pps2++;
					count++;
					ch = toupper(getchar());
				}
				else {
					puts("Error: Invalid character found...Please re-enter");
					error = 1;
					gets(ps2); // clears the buffer
				}
			}
			count = SIZE2 + 1; // exit loop when error or '\n'
		}
		*pps2 = '\0';
		for(count = 0, pps2 = ps2; *pps2 != '\0'; pps2++, count++);
		if(count <= 1 || count > 20)
			puts("\nError: String out of range...Please re-enter");
	}	while(count <=1 || count > 20 || error);

	puts("\nEnter (any) replacement character: ");
	scanf("%c", &c);
	printf("\nReplacement Character: \t%c", c);
	return(c);
}

void strFilter(char *ps1, char *ps2, char c) {
	char temp [SIZE1], *ptemp, *pps1, *pps2;
	
	for(pps1 = ps1, ptemp = temp; *pps1 != '\0'; pps1++, ptemp++)
		*ptemp = *pps1;	
	*ptemp = '\0';
		
	for(pps2 = ps2; *pps2 != '\0'; pps2++)
		for(ptemp = temp; *ptemp != '\0'; ptemp++)
			if(*ptemp == *pps2)
				*ptemp = c;

	puts("\nFiltered String:\t");
	puts(temp);
}

[Reigning Glory]

The program runs fine the first time through. Once it loops on newline, the strings s1 and s2 point to gibberish values. The problem seems to be occuring right after the strFilter() function call, but I have looked at it numerous times, yet I can't seem to locate the trouble spot.

If one of you gentlemen would be kind enough to run it and find the error, it would be appreciated.

[quzah]

Well for starters, you're running off the end of your array:

for(pps1 = ps1; pps1 < ps1 + SIZE1; pps1++) {

After the loop, you set the "pps1" to null. However, after your loop, "pps1" points past the end of your array.

Second:

gets(ps2); // clears the buffer

You should never use gets. They say "never say never". In this case, they're wrong. Never use gets. Ever. Read the FAQ.

Third:

*pps2 = '\0';

You've blown past the end of your array again.

for(pps1 = ps1, ptemp = temp; *pps1 != '\0'; pps1++, ptemp++)
		*ptemp = *pps1;

Why? Why not just use strncpy?

Furthermore, why not just use strchr to find the character you want to replace, then just change where you point. Much quicker.

Quzah.

[Reigning Glory]

Thanks, Quzah.

As per not using the string handling functions, well, the lecturer doesn't want us using any until the next lab. She wants us to use Pure pointer notation for this lab, and I guess that's why some of these assignments are throwing me off.

I'll try doing what you suggested.

[quzah]

Ah. Well you could always write your own versions of the function strchr:

char *mystrchr( const char *s, const char c )
{
    char *p = s;

    if( s == NULL ) return NULL;

    while( *p++ != NULL )
        if( *p == c ) return p;
    return NULL;
}

There ya go.

Quzah.

[Reigning Glory]

Thanks again. In as much as I would like to use your code, I personally have not yet read upto pointer functions, but I'll take it for reference.

I don't know why I found this board so late. Great resource.

On a different note, are you also familiar with Java? Just curious, since I am taking a course in simultaneously with C, and may be C++ references could help me out with my Java projects.

[quzah]

The function is no different than your function, except it returns a pointer to a character.

Here, it returns a pointer to the spot in the string which contains the characer to be replaced.

You could do the exact same thing, using my loop, but replace the if check so that instead of returning, it simply replaces the character.

if( *foo == bar )
    *foo = putmehere;

There isn't really any need to copy the data to a temporary array, unless you just feel like doing it for the heck of it.

On a side note, your function doesn't actually even modify anything, since the string is trashed at the end of the function call. As such, you could fake it with something like this:

void fakeit( char * s, const char c, const char f )
{
    while( s && *s )
    {
        if( *s == c )
            putchar( f );
        else
            putchar( *s );
        s++;
    }
}

Naturally you could use the above with slight modifications to do what you're intending anyway. This is just a cheesy way around it. Visually you will not see anything different had you run my function or your function.

With either one, they'd both display the "string" with the character in question "replaced".

Quzah.

[Reigning Glory]

Alright, I upped SIZE1 by 1, and used SIZE2 - 1 in the userString function. You were right, I was incrementing beyong the last element with my pointers. The program runs smoothly now. Thanks for the input.

[Reigning Glory]

The reason I am using the temp array is because the lab requires that the program only generates the random string once. I could use strcpy instead to assign s1 to temp, but like I said earlier, no string handling functions allowed in this lab. Thus the extra lines of code.

Thanks for the input. I'll refer to them in for the next projects.